Disposing the members that implement IDisposable

In my Dispose methods (like the one below), everytime i want to call someObj.Dispose() i also have a check for someObj!=null.

Is that because of bad design on my part? Is their a cleaner way to ascertain that Dispose of all the members (implementing IDisposable) being used in an object is called without having a risk of NullReference exception ?

protected void Dispose(bool disposing)
        {
            if (disposing)
            {
               if (_splitTradePopupManager != null)
                {
                    _splitTradePopupManager.Dispose();
                }
             }
        }

Thanks for your interest.

I like @Dan Tao's solution, but it's much better as an extension method, imo:

public static void SafeDispose(this IDisposable obj)
{
    if (obj != null)
        obj.Dispose();
}

Now you can just call member.SafeDispose() on any IDisposable in your program without worry. :)

Maybe someone else can chime in on this, but I don't personally think it's a design flaw -- just the safest way to do it.

That said, nothing's stopping you from wrapping your null check and Dispose call in a convenient method:

private void DisposeMember(IDisposable member)
{
    if (member != null)
        member.Dispose();
}

Then your Dispose method could look a bit cleaner:

protected void Dispose(bool disposing)
{
    if (disposing)
    {
        DisposeMember(_splitTradePopupManager);
        DisposeMember(_disposableMember2);
        DisposeMember(_disposableMember3);
    }
}

As an added bonus, this also resolves a potential race condition in your original code. If running in a multithreaded context, the if (_field != null) _field.Dispose() pattern can result in a NullReferenceException when _field is set to null between the check and the disposal (rare, but possible). Passing _field as an argument to a method such as DisposeMember copies the reference to a local variable in the method, eliminating this possibility, unlikely as it is.

Only you know the answer to this one!

Without seeing your entire class it's difficult for anyone else to tell if it's possible that those members will ever be null when Dispose is called.

(Of course, as a general rule it's always possible for a reference type or nullable value type to be null, so it's probably good practice to always include those null checks anyway.)

The only other option I could think of would be to create a DisposeParameter helper method that has an object as it's parameter and only checks if it's null and otherwise Dispose it. That way you'd only need one line of code to dispose of it, but I'm not sure if it would make it more readable.

Try this.

    protected void Dispose(bool disposing) 
    { 
        if (disposing) 
        {
           //for all members.. 
           if (null != member && member is IDisposible) 
            { 
                member.Dispose(); 
            } 
         } 
    }