ASP.NET Web Forms Singleton

Question

Is this way acceptable to do a C# singleton for use in ASP.NET webforms legacy app:

public class FileSingleton
{
    private static readonly Lazy<FileSingleton> lazy =
        new Lazy<FileSingleton>(() => new FileSingleton( new FileRepository()));


    public static FileRepository Repository { get; private set; }

    public static FileSingleton Instance { get { return lazy.Value; } }

    private FileSingleton(FileRepository repository)
    {
        Repository = repository;
    }
}

I created a repository for dealing with files in Azure. It has some overhead, so I don't want to instantiate it on every request. My design is based off of this post.

protected void Application_Start(object sender, EventArgs e) 
{

    HttpContext.Current.Items.Add("fileRepository", FileSingleton.Instance);
}

Does that seem reasonable? Is there a better way to do this?

Answer 1

First of all a bug, Repository property has to be an instance property, not static. Not that big issue because you're working with a singleton but it defeats the purpose of its surrounding code. Don't go to fix this, wait for our refactoring...

Class itself should be sealed, I don't see any chance to extend it (and you probably want to avoid it). Again, wait little bit more...

Note that, according to C# version you're using, you may write read-only properties as:

public FileRepository Repository { get; }    
public static FileSingleton Instance => lazy.Value;

About Application_Start. I do not see surrounding code then I'm guessing but I have few perplexities. HttpContext.Items is usually handy to share data between HTTP modules and/or HTTP handlers. In this case you're storing an instance of FileSingleton, it means that each user has to know that type. In this case they can directly access FileSingleton.Instance.

---

As last note I'd consider to merge FileSingleton and FileRepository (if you own that code and it can be changed). Consider that to access FileRepository methods you need to write something like this:

FileSingleton.Instance.Repository.DoSomething();

Isn't it too verbose? It should be:

FileSingleton.Repository.DoSomething();

Or:

FileRepository.Instance.DoSomething();

If possible I'd go with the last one to do not introduce another type with the only responsibility to keep an instance of something else. If not then I'd refactor your code to:

public static class FileSingleton
{
     private static readonly Lazy<FileRepository> _repository =
        new Lazy<FileRepository>(() => new FileRepository());

     public static FileRepository Instance => _repository.Value;
}