0

I'm using the singleton design pattern for my Business Layer and Data Access Layer classes in C#.

My Class looks as below:

using System;
using System.Data;
using System.Data.Common;
using Microsoft.Practices.EnterpriseLibrary.Data;
using Microsoft.Practices.EnterpriseLibrary.ExceptionHandling;

public class CabBookingDao: IDisposable
{
    private static volatile CabBookingDao _instance;
    private static readonly object SyncRoot = new Object();

    private readonly DatabaseProviderFactory _factory;
    private readonly Database _db;
}

public static CabBookingDao Instance
{
    get
    {
        if (_instance == null)
        {
            lock (SyncRoot)
            {
               if (_instance == null)
                 _instance = new CabBookingDao();
            }
        }
        return _instance;
    }
}

private CabBookingDao()
{
    _factory = new DatabaseProviderFactory();
    _db = _factory.Create("MyDbContext");
}

public void Dispose()
{
   GC.SuppressFinalize(this);
}

Here, I'm trying to Implement IDisposable for this class and I'm getting Warning from VS Code Analyzer like:

CA1063 Implement IDisposable correctly Provide an overridable implementation of Dispose(bool) on 'CabBookingDao' or mark the type as sealed. A call to Dispose(false) should only clean up native resources. A call to Dispose(true) should clean up both managed and native resources CabBookingDao.cs

By this warning, I understood that I suppose to create a method as below:

protected virtual void Dispose(bool disposing)
{
    if (disposing) 
    {
        // free managed resources
    }
    // free native resources if there are any.
}

But, I'm confused about what objects do I need to dispose here? If I dispose _instance, will it affect the Singleton behavior?

55SK55
  • 621
  • 2
  • 8
  • 23
  • `But, I'm confused about what objects do I need to dispose here?` Non-static members of the object that aren't shared with other objects. – mjwills Aug 10 '18 at 12:40
  • Do you mean, In my case, `_factory` and `_db`? – 55SK55 Aug 10 '18 at 12:46
  • 1
    _Probably_, yes. It depends on a number of factors. For example, you may have a IoC container, which _may_ mean you don't need the `Dispose` at all (since the container will handle it for you). But my first comment is a reasonable "general rule". – mjwills Aug 10 '18 at 12:48
  • To be clear, I wouldn't use your `lock` technique - I'd just use `Lazy` instead (well technically I'd use https://stackoverflow.com/a/42567351/34092). – mjwills Aug 10 '18 at 12:51
  • @55SK55 I suggest dropping the double-check lazy initialization pattern, it's nearly impossible to deal right with volatile fields. There are some well-tested framework constructions for this purpose: [Lazy](https://msdn.microsoft.com/en-us/library/dd642331(v=vs.110).aspx) or [LazyInitializer](https://msdn.microsoft.com/en-us/library/system.threading.lazyinitializer(v=vs.110).aspx). – Adam Simon Aug 10 '18 at 12:52
  • 1
    I'd suggest something like `private static Lazy _instance = new Lazy(() => new CabBookingDao());` Again, I'd use `LazyWithNoExceptionCaching` instead to be safer though - https://stackoverflow.com/questions/34529533/system-lazyt-with-different-thread-safety-mode/42567351#42567351 . – mjwills Aug 10 '18 at 13:05
  • 1
    If you shut it down and recreate it's **not** a singleton, it's just an object. You don't implement a disposable singleton. Singletons go out of scope automatically when the `AppDomain` is shutdown (which happens to be when you program is terminating) and typically you only have **one** unless you are making child `AppDomain`s –  Aug 10 '18 at 13:05

0 Answers0