Disposing object in C#

Question

I have written the following class:

public class CoupleFrames
{
    public CoupleFrames(ColorImageFrame cif, Bitmap df)
    {
        this.colorFrame = cif;
        this.desktopFrame = df;
    }

    public ColorImageFrame colorFrame;
    public Bitmap desktopFrame;
}

Now I'm using the following code for disposing the variables.

CoupleFrames cf = new CoupleFrames(frame1, frame2);
// some code...
cf.colorFrame.Dispose();
cf.desktopFrame.Dispose();

I'm not sure that this is the correct way. Someone can suggest me the correct way for disposing the entire object?

This depends if you want the type to take ownership of the variables passed in to the constructor; if not, use a using for the variables, otherwise implement IDisposable on the type and have it manage the resources. — Grant Thomas
– Grant Thomas, Commented Jun 26, 2013 at 9:16

Jon Skeet · Accepted Answer · 2017-10-25 15:53:09Z

17

I'm not sure that this is the correct way. Someone can suggest me the correct way for disposing the entire object?

Sure - you should make CoupleFrames implement IDisposable, and its Dispose method should dispose of the objects it "owns". For example:

public sealed class CoupleFrames : IDisposable
{
    private readonly ColorImageFrame colorFrame;
    private readonly Bitmap desktopFrame;

    public CoupleFrames(ColorImageFrame cif, Bitmap df)
    {
        // TODO: Argument validation, unless it's valid for these parameters
        // to be null, in which case the Dispose method would need to be careful.
        this.colorFrame = cif;
        this.desktopFrame = df;
    }

    public void Dispose()
    {
        colorFrame.Dispose();
        desktopFrame.Dispose();
    }
}

A few points to note:

You should make sure it's clear that the CoupleFrame really "owns" these constituent objects. Disposal relies on a clear ownership model
If CoupleFrame isn't sealed (and can't be) you may need to go into a more complicated pattern with virtual methods and finalizers. It can get very complicated, and you should read the advice given here by Joe Duffy et al. If your class is sealed, a lot of that complexity goes away
Public fields are generally a bad idea (in terms of encapsulation), which is why I've made them private here. I've also made them readonly, as if they can be changed later you need to think about whether changing them should dispose of the previously-referenced object etc.
By making CoupleFrame implement IDisposable, you're basically telling all clients that they should dispose of any instance they own. If you're not happy with imposing that burden, you need to rethink the design a bit.

edited Oct 25, 2017 at 15:53

answered Jun 26, 2013 at 9:14

Jon Skeet

1.5m893 gold badges9.3k silver badges9.3k bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

ken2k Over a year ago

As CoupleFrames doesn't instantiate those 2 private members, we should check for NRE in the dispose method.

Jon Skeet Over a year ago

@ken2k: I'd actually put non-nullity validation in the constructor. Will edit to indicate that.

GVillani82 Over a year ago

@ken2k So, I need also to test if the members are null?

Jon Skeet Over a year ago

@Joseph82: You need to test somewhere - is it valid for null values to be passed into the constructor? If not, throw an exception there. If it is, check before you try to dispose.

rexcfnghk · Accepted Answer · 2013-06-26 09:48:20Z

3

I would implement the Dispose pattern

public class CoupleFrames : IDisposable
{
    public CoupleFrames(ColorImageFrame cif, Bitmap df)
    {
        this.colorFrame = cif;
        this.desktopFrame = df;
    }

    public ColorImageFrame colorFrame;
    public Bitmap desktopFrame;

    private bool disposed;

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

    protected virtual void Dispose(bool disposing)
    {
        if (disposed)
        {
            return;
        }
        if (disposing)
        {
            colorFrame.Dispose();
            desktopFrame.Dispose();
        }
        disposed = true;
    }
}

answered Jun 26, 2013 at 9:48

rexcfnghk

15.7k1 gold badge34 silver badges59 bronze badges

Comments

Sri Harsha Chilakapati · Accepted Answer · 2013-06-26 09:17:34Z

-1

You can use the IDisposable interface.

public class CoupleFrames : IDisposable
{
    ....

    public void Dispose()
    {
        // Your disposing code here
    }

    ~CoupleFrames()
    {
        Dispose();
    }
}

You can use the destructor to call the Dispose method since the object can sometimes be deleted by the GC.

answered Jun 26, 2013 at 9:17

Sri Harsha Chilakapati

12k6 gold badges54 silver badges91 bronze badges

Comments

Avi Turner · Accepted Answer · 2013-06-26 09:13:59Z

-3

Make CoupleFrames Implement the Idisposable Interface.

public class CoupleFrames : IDisposable
{
    public CoupleFrames(ColorImageFrame cif, Bitmap df)
    {
        this.colorFrame = cif;
        this.desktopFrame = df;
    }

    public ColorImageFrame colorFrame;
    public Bitmap desktopFrame;

    public void Dispose()
    {
        cf.colorFrame.Dispose();
        cf.desktopFrame.Dispose();
    }

answered Jun 26, 2013 at 9:13

Avi Turner

10.5k8 gold badges51 silver badges79 bronze badges

Collectives™ on Stack Overflow

Disposing object in C#

4 Answers 4

4 Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

4 Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related