How do I dispose of ToolStripDropDown?

Question

I ran the Code Analysis option on my VS2012 project and it discovered the following;

CA1001 Types that own disposable fields should be disposable Implement IDisposable on 'DataGridViewColumnSelector' because it creates members of the following IDisposable types: 'CheckedListBox', 'ToolStripDropDown'. If 'DataGridViewColumnSelector' has previously shipped, adding new members that implement IDisposable to this type is considered a breaking change to existing consumers. DataGridColSelector DataGridViewColumnSelector.cs

I made my class, DataGridViewColumnSelector inherit from IDisposable and am wondering what to put in the dispose method

Update :

Here is my attempt. Code Analysis has stopped complaing since I made the class sealed. I still aren't sure I am "doing it right "

    public sealed class DataGridViewColumnSelector  :IDisposable 
{
    private DataGridView mDataGridView = null;
    private CheckedListBox mCheckedListBox;
    private ToolStripDropDown mPopup;

    public delegate void CustomRightClickDelegate(object sender, MouseEventArgs e);

    public event CustomRightClickDelegate GridRightClickEvent;
    /// <summary>
    /// The max height of the popup
    /// </summary>
    public int MaxHeight = 300;
    /// <summary>
    /// The width of the popup
    /// </summary>
    public int Width = 200;

    public DataGridView DataGridView
    {
        get { return this.mDataGridView; }
        set
        {
            if (this.mDataGridView != null) this.mDataGridView.MouseDown -= this.mDataGridView_MouseDown;
            this.mDataGridView = value;
            if (this.mDataGridView != null) this.mDataGridView.MouseDown += this.mDataGridView_MouseDown;
        }
    }



    void mDataGridView_MouseDown(object sender, MouseEventArgs  e)
    {
        if (e.Button == MouseButtons.Right)
        { 

            if( this.mDataGridView.HitTest(e.X, e.Y).Type == DataGridViewHitTestType.ColumnHeader)
            {
                this.mCheckedListBox.Items.Clear();
                foreach (DataGridViewColumn c in this.mDataGridView.Columns)
                {
                    this.mCheckedListBox.Items.Add(c.HeaderText, c.Visible);
                }
                int PreferredHeight = (this.mCheckedListBox.Items.Count*20);
                this.mCheckedListBox.Height = (PreferredHeight < this.MaxHeight) ? PreferredHeight : this.MaxHeight;
                this.mCheckedListBox.Width = this.Width;
                this.mPopup.Show(this.mDataGridView.PointToScreen(new Point(e.X, e.Y)));
            }
            else
            {
                if (this.GridRightClickEvent != null)
                {
                    this.GridRightClickEvent.Invoke(sender, e);
                }

            }
        }
    }

    public DataGridViewColumnSelector()
    {
        this.mCheckedListBox = new CheckedListBox();
        this.mCheckedListBox.CheckOnClick = true;
        this.mCheckedListBox.ItemCheck += new ItemCheckEventHandler(this.mCheckedListBox_ItemCheck);

        ToolStripControlHost mControlHost = new ToolStripControlHost(this.mCheckedListBox);
        mControlHost.Padding = Padding.Empty;
        mControlHost.Margin = Padding.Empty;
        mControlHost.AutoSize = false;

        this.mPopup = new ToolStripDropDown();
        this.mPopup.Padding = Padding.Empty;
        this.mPopup.Items.Add(mControlHost);
    }

    public DataGridViewColumnSelector(DataGridView dgv)
        : this()
    {
        this.DataGridView = dgv;
    }

    void mCheckedListBox_ItemCheck(object sender, ItemCheckEventArgs e)
    {
        this.mDataGridView.Columns[e.Index].Visible = (e.NewValue == CheckState.Checked);
    }

    public void Dispose()
    {
        //http://stackoverflow.com/questions/6826958/c-toolstripdropdown-doesnt-dispose-destroyhandle
        // http://msdn.microsoft.com/en-au/library/b1yfkh5e%28v=vs.71%29.aspx 
        // Kirsten says I dont feel sure about what I am doing here.

        mCheckedListBox.Dispose(  );
        mPopup.Dispose(  );
        GC.SuppressFinalize(this);

    }
}

That depends on how you implemented `IDisposable`, and how often you create a new instance. Try showing your code, and how you use it. — Jens Kloster, Apr 22 '13 at 05:53
You are disposing the right controls in your method. I would suggest you to make a null check before disposing a control and set it to null afterwards (just to ensure GC is really recognizing that you don't need this object anymore).If you don't really need it, don't call GC methods as GC works automatically. — , Apr 22 '13 at 07:02

Jens Kloster · Accepted Answer · 2013-04-22T18:56:52.223

1

You shold call dispose then your form disposes.

Also. your implementation of IDisposable is lacking some vital things.

1) You should make sure there are no events subscribed to your custom event. That could end up giving your application a memoryleak.

//in dispose
GridRightClickEvent = null

2) MSDN has a best practice for implemeting IDisposable

public sealed class DataGridViewColumnSelector  : IDisposable
{
  //removed: ~DataGridViewColumnSelector  (){ Dispose(false); /*destructor*/ }

  //class context omitted

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

  private void Dispose(bool disposing)
  {
    if(disposing)
    {
      //kill the reference - do not dispose the object. 
      //it was *not* created here, os it should *not* be disposed here
      mDataGridView = null; 

      //makes sure no outside object has a reference
      //to the event - thus keeping it alive when it should be garbagecollected
      GridRightClickEvent = null; 

      if(mCheckedListBox != null) mCheckedListBox.Dispose();

      if(mPopup != null) mPopup.Dispose();

      if(mControlHost  != null) mControlHost .Dispose();

    }
  }
}

edited Apr 22 '13 at 18:56

answered Apr 22 '13 at 08:08

Jens Kloster

11,099
5
40
54

Implementing a finalizer (aka destructor) is **not** a best practice. It is only required when the class stores unmanaged resources, that should *never* be the case with code like this. The strongest indication of making this mistake is when the Dispose(bool) method doesn't do anything useful when *disposing* is false. Like in your snippet. – Hans Passant Apr 22 '13 at 10:50
@HansPassant That makes sence. I edited the code exampel - removing the destructor – Jens Kloster Apr 22 '13 at 11:12
So what goes below //handle your dispose here ? is it `GridRightClickEvent = null; mCheckedListBox.Dispose(); mPopup.Dispose();` – Kirsten Apr 22 '13 at 15:55
@kirsteng I have updated my example to show how I would handle the Dispose method. – Jens Kloster Apr 22 '13 at 18:57
Thanks - what is mControlHost ? I don't have this in my code. – Kirsten Apr 22 '13 at 20:23
@kirsteng in your constructor. But it Will properly be disposed when mPopup does. – Jens Kloster Apr 22 '13 at 20:46
it is private to the initializer so i have removed it – Kirsten Apr 23 '13 at 10:13

How do I dispose of ToolStripDropDown?

1 Answers1