C# - I know, I know, another question about Dispose (more related to design)!

Question

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. :)

Answer 1

You don't show the code for SetDataSource but this piece of code is only going to work if SetDataSource immediately consumes the contents of the DataSet and does not store a reference to it:

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

As for the rest of your code and question (quoting a deleted answer):

---

you don't have anything that needs to be finalized. You might have noticed that when you found out that you had no use for the disposing argument. All you need is this:

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

---

The general rule for Finalizers is: When in doubt, Don't.
You will only need one when you directly use an unmanaged resource.

Answer 2

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.

Answer 3

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.]

Answer 4

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...