tags:

views:

337

answers:

3

Possible Duplicate:
Should I Dispose() DataSet and DataTable?

OP comment: I would like to say that the "Should I Dispose() DataSet and DataTable?" link isn't a possible solution. It's a good link, but this is more design related. Instead, ignore that the exposed property is a DataSet and replace it with something that should be disposed. The same question would apply there.

I'm messing around with Crystal Reports, and I have a "ReportData" class. In other words, this class encapsulates the "Filling" of the DataSet I will use.

public class ReportData
{
    private DataSet1 m_DS = null; // Notice this disposable member variable

    public ReportData( ... some parameters ...)
    {
        m_DS = new DataSet1();
        // Plus some other manipulation on m_DS
    }

    public DataSet1 GetDataSet
    {
        get
        {
            return m_DS;
        }
    }

    // Everything else is pretty much private.
    // This class is here to generate my DataSet
}

Here is how it would be used by some other class:

private void SetReportDataSource()
{
    DataSet1 ds = m_RptData.GetDataSet;
    m_Rpt.SetDataSource(ds);
}

I'm pretty much learning C# on the fly (read a couple of chapters in an intro book, and just went at it, googling everything along the way). From what I understand, if it implements IDisposable, you better Dispose it. A DataSet implements IDisposable, so we need to Dispose it.

Here's where the design part comes in:

Question 1a: Do I make my ReportData class IDisposable?

In other words, it looks like I could just do this and be done with it:

private void SetReportDataSource()
{
    using (DataSet1 ds = m_RptData.GetDataSet)
    {
        m_Rpt.SetDataSource(ds);
    }
}

Question 1b: Should I be more defensive in some way?

I don't know, I guess I'm really, really trying to ensure that it gets disposed. For example, taking my SetReportDatsSource function as an example. I used "using", but someone else may use the class and forget to add the using or call Dispose in some way. Therefore, I go to my ReportData class:

public class ReportData : IDisposable
{
    private DataSet1 m_DS = null; // Notice this disposable member variable
    private bool m_IsDisposed = false; // This is new!

    public ReportData( ... some parameters ...)
    {
        m_DS = new DataSet1();
        // Plus some other manipulation on m_DS
    }

    // New code here (up until the GetDataSet property)
    ~ReportData()
    {
        Dispose(false);
    }

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

    protected virtual void Dispose(bool disposing)
    {
        if (m_IsDisposed == true)
        {
            return;
        }

        if (m_DS != null)
        {
            m_DS.Dispose();
            m_DS = null;
        }

        m_IsDisposed = true;
    }

    // Done with new code

    public DataSet1 GetDataSet
    {
        get
        {
            return m_DS;
        }
    }

    // Everything else is pretty much private.
    // This class is here to generate my DataSet
}

Now, let's go back to the calling class, we still have:

private void SetReportDataSource()
{
    using (DataSet1 ds = m_RptData.GetDataSet)
    {
        m_Rpt.SetDataSource(ds);
    }
}

But, I made ReportData (m_RptData) disposable now too! So, we are going to want to dispose of that! And because it's a member variable (and I can't just use "using" in the way I'm using it with SetReportDataSource), you start thinking about making this calling class IDisposable, so I can have:

    protected virtual void Dispose(bool disposing)
    {
        if (m_IsDisposed == true)
        {
            return;
        }

        if (m_ReportData != null)
        {
            m_ReportData.Dispose();
            m_ReportData = null;
        }

        m_IsDisposed = true;
    }

So, now this class has the destructor/finalizer and its public Dispose method, disposing of ReportData. We're guaranteed that we dispose of the DataSet!

But then again, this will result in DataSet's Dispose method getting called twice. Because the SetReportDataSource function disposes the exposed DataSet, and the ReportData class is also disposing the same thing (and there is not an easy way to determine if someone disposed of the exposed DataSet).

Seems kind of nutty. It looks like to me that I:

a) May be overthinking this (or just trying to be really defensive, which is good)!

b) Might be jumping through a bunch of hoops.

Maybe the rule should be: If my class is going to expose it, the calling function should be responsible for disposing it.

The only issue I see with that (and that's why I posted here):

In between the time that ReportData member variable is instantiated, and SetReportDataSource is called, some error/exception may occur. So, we never get a chance to do the "using" (inside of SetReportDataSource) on our DataSet. But that dataset was constructed with ReportData (and we want to call dispose on it!)

So, now we're back to making ReportData IDisposable, because we are going to at least need some public "CleanUp" function ... ok, I'm done. :)

+3  A: 

Please see: Should I Dispose() DataSet and DataTable?

As has come up several times on SO, if an object it implements IDisposable then you should call Dispose(). [There are a few places in the .NET framework where the original designers, presumably thouight they would require Dispose() but then didn't need it. But the safest and correct thing to do would be to call Dispose regardless.]

Mitch Wheat
This is a useful link, thanks. But let's ignore the fact that this is a DataSet. It could be something else, I don't know, a Font (that I exposed). The same discussion would apply.Admittedly, I'm in the mode of: if it has a Dispose method, I want to call it. As opposed to: It has a Dispose method, let me google the world to see if it really does anything. And now, I need to get a bit better and figure out how these people are debugging into the C# source code to determine that the Dispose really is useless ... and ... phew!But thanks, good link. And a +1 for you.
JustLooking
+4  A: 

If you are holding onto something that implements IDisposable, the simplest pattern is to implement IDisposable yourself, and call the Dispose methods on all IDisposables you own. Being more defensive than doesn't seem feasible; if somebody using your class forgets to call your Dispose method, then how do you ever know they are done using you?

Edit: On second thoughts, a Finalize call tells you that your client is done using you; they have no more references to you...

Tarydon
A: 

Answer 1A: I would recommend the using syntax, the reason for that is it seems to be the standard for handling things of this nature.

Answer 1B: If someone else is using your class, I assume you mean extending, it is on the implementer to handle any changes he sees fit. According to the docs on Dispose()

If an object's Dispose method is called more than once, the object must ignore all calls after the first one. The object must not throw an exception if its Dispose method is called multiple times. Dispose can throw an exception if an error occurs because a resource has already been freed and Dispose had not been called previously. (http://msdn.microsoft.com/en-us/library/system.idisposable.dispose%28VS.71%29.aspx)

The garbage collector will at some point get a hold of your object and handle it. Barring catastrophe.

Woot4Moo
A point for responding (I fear I may have to re-post this question using something other than DataSet). As for 1B, I did not mean extending. I meant the calling class instantiating ReportData and then later on using it (one of the issues, in my mind, was instantiating ... exception/error happening before I ever get to use the using).
JustLooking
Well in that case an exception would have happened and it won't matter. The GC should get a hold of it since there are no valid references available
Woot4Moo
(Another point for a response) Actually, what I mean is: ReportData gets instantiated (thus, DataSet does - DataSet is Disposable). Everything is OK. Later on, in some other event, an error occurs. We never get to call SetReportDataSource because of this error. Now, we need to dispose of ReportData/DataSet. I'm not talking about an exception during the actual instantiation. We are relying on just using the "using' in SetDataSource (to dispose of the DataSet), but if an error happens at some point between the completed instantiation and SetDataSource, then what?
JustLooking
Assuming the exception propagates all the way up and the program terminates. The Garbage collector should take care of that issue, however there are some cases where a memory leak is possible. However, if you just swallow exceptions than there will be an issue.
Woot4Moo