CODEWITHSUNDEEP
c#winformscode-analysis
Kirsten
Kirsten

Reputation: 18076

Code Analysis finds CA2213 error in designer code

When I run Code Analysis on my c# winforms application I get the following warning;

CA2213 Disposable fields should be disposed 'LogEntryForm' contains field 'LogEntryForm._changeValuesNavigator' that is of IDisposable type: 'DynamicBindingNavigator'. Change the Dispose method on 'LogEntryForm' to call Dispose or Close on this field. UI LogEntryForm.Designer.cs 15

the code in question is 

  partial class LogEntryForm
    {
        /// <summary>
        /// Required designer variable.
        /// </summary>
        private System.ComponentModel.IContainer components = null;
        /// <summary>
        /// Clean up any resources being used.
        /// </summary>
        /// <param name="disposing">true if managed resources should be disposed; otherwise, false.</param>
        protected override void Dispose(bool disposing)
        {
            if (disposing && (components != null))
            {
                components.Dispose();
            }
            base.Dispose(disposing);
        }

How do I stop the designer creating code that fails code analysis?

Update: I tried moving the Dispose to the form code as

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

    private void Dispose(bool disposing)
    {
        if (disposing)
        {


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


             if (_changeOperationsNavigator != null) _changeOperationsNavigator.Dispose();
             if (_changeValuesNavigator != null) _changeValuesNavigator.Dispose();


        }
    }

but now Code Analysis complains

CA2213 Disposable fields should be disposed 'LogEntryForm' contains field 'LogEntryForm.components' that is of IDisposable type: 'IContainer'. Change the Dispose method on 'LogEntryForm' to call Dispose or Close on this field. UI LogEntryForm.cs 214

[update] After studying Mathew's answer I changed the form to only have one Dispose method 

  private void Dispose(bool disposing)
        {
            if (disposing)
            {
                 if (_changeOperationsNavigator != null) _changeOperationsNavigator.Dispose();
                 if (_changeValuesNavigator != null) _changeValuesNavigator.Dispose();
                 if (components != null)
                     components.Dispose();

            }
        }

I no longer get the warning.

Upvotes: 4

Views: 5248

Answers (1)

Matthew Watson
Matthew Watson

Reputation: 109547

I always found it irksome that the designer puts the Dispose() method into the "Designer.cs" file. If you look at the generated code, you'll see that the Dispose() method is placed before the region that says 

#region Windows Form Designer generated code

Whenever I want to change Dispose() I always move it out of the "Designer.cs" file to the main ".cs" file, because I don't like to have manually-written methods in the "Designer.cs" file.

Given that the warning seems to be real, I would do the same in your case. Move the "Dispose()" method to your main ".cs" file and either dispose the thing that CA is complaining about or (if you're sure it's ok) suppress the warning.

I don't know how to fix the original problem (that the code generated by the designer doesn't dispose the object in question) - but the designer never seems to touch Dispose() in any circumstances (other than when it is first generated), so I guess fixing it yourself is the only option.

[EDIT]

I also notice that your dispose method seems to be missing some boilerplate code that's normally automatically generated.

Usually, Dispose() should look like this:

protected override void Dispose(bool disposing)
{
    if (disposing && (components != null))
    {
        components.Dispose();
    }
    base.Dispose(disposing);
}

(along with any code you add yourself.)

Can you add that code in and see what happens? Note how components matches the name of the field mentioned in the error message. I think yours should therefore look like this:

private void Dispose(bool disposing)
{
    if (disposing)
    {
        //makes sure no outside object has a reference
        //to the event - thus keeping it alive when it should be garbagecollected

        if (_changeOperationsNavigator != null) _changeOperationsNavigator.Dispose();
        if (_changeValuesNavigator != null) _changeValuesNavigator.Dispose();

        if (components != null)
            components.Dispose();
    }
}

(It's still good practise to move the Dispose() method out of Designer.cs if you modify it. The odd thing however is why aren't those lines of code in there?)

Upvotes: 4

Related Questions