Winforms: Is it safe to delete the code in the automatically created Dispose() method created by Visual Studio for Windows Forms

Question

Background

In Visual Studio 2008 Create a new Windows Forms Application, this will create a skeleton project with a class called "Form1". VS2008 automatically creates a Dispose() method.

    /// <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);
    }

I wandered into a coworker's office (a senior developer) - great guy, smart, good design skills for a chat - but I noticed what he was typing - as he navigated the codebase he deleted this section of the Dispose() methods that VS2008 created for the forms.

        if (disposing && (components != null))
        {
            components.Dispose();
        }

So, I asked him why and he said it wasn't necessary to keep it in.

The Questions

• Is it safe to delete this code?
• What are the pros/cons of leaving it in or removing it?

Answer 1

No it is not safe to delete this code. Several components depend on this pattern to be in effect in order to properly release unmanaged resources they are holding onto. If you delete this code likely your application will work 95% of the time. The 5% cases though will show up as resource leaks, potentially memory leaks, non-deterministic bugs, ... in general, hard to track down problems.

The best approach for dealing with a disposable resource is to dispose the instant you are no longer using it. Following this practice will save you many headaches down the road. Deleting this code is taking the exact opposite approach.

This post on SO contains a more detailed explanation: When should I dispose my objects in .NET?

Answer 2

Absolutely don't delete it if there is the slightest chance of you ever using something like a System.Windows.Forms.Timer on the form. And there always is the chance of this.

It's this code which will dispose the timer when the form closes, and if you don't dispose the timer then it will keep running. I have taken over several bits of code which have had this type of bug introduced (pre .NET 2.0 this mechanism didn't work so well), and the resulting intermittent perf problems caused by timers running long after the forms they should update have closed can be a real pain.

There is no realistic benefit to deleting this - if no controls have added themselves to the components list then components.Dispose() is going to be trivial to run. If controls have added themselves to the list then that's because they ought to be disposed.