Good day! I have this question. Suppose I have a class that implements the IDisposable interface. This class has a managed resource that also implements this interface. Here is the minimum code for my class.

class MyClass : IDisposable { private SomeContext _context = new SomeContext(); public void Dispose() { if(_context != null) _context.Dispose(); } } 

And then I had a question. How correct is this implementation? More precisely, how wrong is it and how does it threaten? (wherever I looked I saw a more complex implementation of the Dispose pattern). It seems that there is only one IDisposable field (it is assumed that it correctly implements this pattern in its internals), and in order to release the resource correctly, it is sufficient to simply call the Dispose method for the _context field at the right time. What am I wrong? Is such a simple implementation applicable to the practitioner and why is it generally bad?

  • Now I’ll find a duplicate :) - VladD
  • four
  • @VladD, and why not bummed? - Grundy
  • @Grundy, these are not questions. There is about IDisposable itself, and here about a specific implementation. - Qwertiy
  • 2
    Dispose Pattern of the Framework Design Guidelines (required to read in full). - Alexander Petrov

2 answers 2

Yes, in this scheme everything is correct. There are no unmanaged resources, the finalizer is not needed. Dispose invokes the call further, i.e. it does its job.

In general, I recommend reading: http://sergeyteplyakov.blogspot.ru/2011/09/dispose-pattern.html , especially the section Simplified version of the Dispose pattern, which is very similar to this question.

    All right, but you can remove the check for null .

    If the class was constructed, then _context not null , because new cannot return null .

    Total:

     class MyClass : IDisposable { SomeContext _context = new SomeContext(); public void Dispose() => _context.Dispose(); } 

    Please note, it is sometimes appropriate to use such a structure:

     class MyClass : IDisposable { bool isDisposed = false; void EnsureSelfAlive() { if (isDisposed) throw new ObjectDisposedException(); } SomeContext _context = new SomeContext(); public void Dispose() { isDisposed = true; _context.Dispose(); } } 

    and in each public method at the beginning call EnsureSelfAlive() . Allows you to catch a certain number of errors.

    isDisposed is no need to check isDisposed in Dispose , _context should be able to “survive” multiple calls to _context.Dispose() .

    • why? And if someone somewhere assigns the value null? :) - Grundy
    • @Grundy: Well, if it is :) In the current code is not needed. - VladD
    • Um .. and if there is an exception in the new SomeContext() ? - Qwertiy
    • @Qwertiy, Then the exception will fly up, it will not be intercepted anywhere, and the main class will not be constructed - Grundy
    • 2
      @Qwertiy: The main thing, of course, is the semantic bonus. But even for example, we can already close the desired file, or unsubscribe from some event or something else, and be inoperable. Well, or just destroy the internal invariant. In this situation, we simply cannot continue to work, and we should throw the correct exception, and not crash due to a NullReferenceException. - VladD