How to dispose of an object that is iterated through via its Next property?

Question

I have an object that uses some underlying native resources, and has a pointer to the next instance, which I iterate through similar to:

MyObject begin = null;

try
{
    begin = GetFirst();

    while (begin != null)
    {
        MyObject next = begin.Next();
        // do something with begin
        begin.Dispose();
        begin = next;
    }
}
finally
{    
    if (begin != null)
    {
        begin.Dispose();
    }
}

I get the code analysis problem:

CA2202: Microsoft.Usage: Object 'begin' can be disposed more than once in method 'x()'. To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.

Any idea how I can get rid of this error without suppressing it?

Answer 1

It certainly seems to me that your last block of code is unnecessary. If begin != null, then your while loop should have continued, no?

UPDATE: It looks like you're trying to ensure the last obtained value for begin is disposed in case an exception is thrown. Try this:

MyObject begin = GetFirst();

while (begin != null)
{
    MyObject next;
    using (begin)
    {
        next = begin.Next();
        // do something with begin
    }

    begin = next;
}

Note that in the above suggestion, it could actually still happen that you end up with an undisposed object: the last value assigned to next, before the end of the using block. This scenario wasn't covered in your original question, so I haven't addressed it in the above suggestion. It's something to consider, though, if that's a potential problem.

Answer 2

It appears that Code Analysis considers it possible for an exception to occur during the Dispose() method. Were that to be the case, you would enter the finally block with an already-disposed-albeit-non-null reference to begin.

Note that I would only prefer this approach to @Dan's if you plan to wrap the call to begin.Dispose() in additional error trapping and handling. IMO, Dan's solution is more elegant.

Here is a try-finally approach that removes the warning:

MyObject begin = GetFirst();
MyObject next = null;

while (begin != null)
{
    try
    {
        next = begin.Next();
        // do something with begin
    }
    finally
    {
        begin.Dispose();
        begin = next;
    }
}

Answer 3

You obviously have some mechanism for identifying the first item in the chain, perhaps some other object or some static that is storing the first item?

How about in your code that is originally calling dispose:

GetFirst().Dispose();

Then the only responsibility for the dispose method is to dispose the current item and it's children:

public void Dispose()
{
    if (Next() != null)
    {
        Next().Dispose();
    }
}

This eliminates any need to loop within the dispose method. I would also take a look at the dispose pattern