Mass Dispose, better way?

Question

So i am currently disposing many objects when i close my form. Even though it probably disposes it automatically. But still i prefer to follow the "rules" in disposing, hopefully it will stick and help prevent mistakes.

So here is how i currently dispose, which works.

        if (connect == true)
        {
            Waloop.Dispose();
            connect = false;
            UninitializeCall();
            DropCall();

        }
        if (KeySend.Checked || KeyReceive.Checked)
        {
            m_mouseListener.Dispose();
            k_listener.Dispose();

        }
        if (NAudio.Wave.AsioOut.isSupported())
        {
            Aut.Dispose();
        }

        if (Wasout != null)
        {
            Wasout.Dispose();
        }
        if (SendStream != null)
        {
            SendStream.Dispose();
        }

So basically, the first is if a bool is true, meaning if it isn´t those can be ignore, as they haven´t been made i think.

The others are just ways for me to dispose if it´s there. but it´s not a very good way, i would like to have it in 1 big function, meaning.

Dispose if it´s NOT disposed. or something. I know that many of them has the "isdisposed" bool, so it should be possible if i can check every object, and dispose if it´s false.

Answer 1

How about a helper method which takes objects which implement IDisposable as params?

void DisposeAll(params IDisposable[] disposables)
{
  foreach (IDisposable id in disposables)
  {
    if (id != null) id.Dispose();
  }
}

When you want to dispose multiple objects, call the method with whatever objects you want to dispose.

this.DisposeAll(Wasout, SendStream, m_mouseListener, k_listener);

If you want to avoid calling them explicity, then store them all in a List<>:

private List<IDisposable> _disposables;

void DisposeAll() {
  foreach(IDisposable id in _disposables) {
    if(id != null) id.Dispose();
  }
}

Answer 2

You can implement a Disposer class, that will do the work for you, along these lines:

public class Disposer
{
   private List<IDisposable> disposables = new List<IDisposable>();

   public void Register(IDisposable item)
   {
      disposables.Add(item);
   }

   public void Unregister(IDisposable item)
   {
      disposables.Remove(item);
   }

   public void DisposeAll()
   {
      foreach (IDisposable item in disposables)
      {
        item.Dispose();
      }
      disposables.Clear();
   }
}

Then, instead of the ugly code in your main class, you can have something like:

public class Main
{
   //member field
   private Disposer m_disposer;

   //constructor
   public Main()
   {
       ....
       m_disposer = new Disposer();
       //register any available disposables
       disposer.Register(m_mouseListener);
       disposer.Register(k_listener);
   }

   ...

   public bool Connect()
   {
       ...
       if (isConnected)
       {
           Waloop = ...
           Wasout = ...
           // register additional disposables as they are created
           disposer.Register(Waloop);
           disposer.Register(Wasout);
       }
   }

   ...

   public void Close()
   {
     //disposal
     disposer.DisposeAll();
   }
}

Answer 3

I suggest you use the using statement. So with your code, it would look something like this:

  using (WaloopClass Waloop = new WaloopClass())
  {
        // Some other code here I know nothing about.

        connect = false; // Testing the current value of connect is redundant.
        UninitializeCall();
        DropCall();
  }

Note there is now no need to explicitly Dispose Waloop, as it happens automatically at the end of the using statement.

This will help to structure your code, and makes the scope of the Waloop much clearer.

Answer 4

I am going to suppose that the only problem you’re trying to solve is how to write the following in a nicer way:

if (Wasout != null)
    Wasout.Dispose();
if (SendStream != null)
    SendStream.Dispose();

This is a lot of logic already implemented by the using keyword. using checks that the variable is not null before calling Dispose() for you. Also, using guarantees that thrown exceptions (perhap by Wasout.Dispose()) will not interrupt the attempts to call Dispose() on the other listed objects (such as SendStream). It seems that using was intended to allow management of resources based on scoping rules: using using as an alternative way to write o.Dispose() may be considered an abuse of the language. However, the benefits of using’s behavior and the concision it enables are quite valuable. Thus, I recommend to replace such mass statically-written batches of the “if (o != null) o.Dispose()” with an “empty” using:

using (
    IDisposable _Wasout = Wasout,
    _SendStream = SendStream)
{}

Note that the order that Dispose() is called in is in reverse of how objects are listed in the using block. This follows the pattern of cleaning up objects in reverse of their instantiation order. (The idea is that an object instantiated later may refer to an object instantiated earlier. E.g., if you are using a ladder to climb a house, you might want to keep the ladder around so that you can climb back down before putting it away—the ladder gets instantiated first and cleaned up last. Uhm, analogies… but, basically, the above is shorthand for nested using. And the unlike objects can be smashed into the same using block by writing the using in terms of IDisposable.)

dotnetfiddle of using managing exceptions.