views:

126

answers:

7

I have some legacy code in C#, .Net 2.0 where someone dragged OpenFileDialog and SaveFileDialaog from the toolbox onto the form. This results in the open and save dialogs being instantiated when their parent dialog is created and remaining until the parent dialog is closed. The open/save forms are really only used in one place each for a support file that is not always opened/saved during the life of the parent dialog; this seems inefficient to me. What I would do is:

    using (OpenFileDialog openDlg = new OpenFileDialog() )
    {
        //initialize openDlg...

        if (openDlg.ShowDialog() != DialogResult.OK)
        {
            //handle OK result
        }
    }

That way the OpenFileDialog will be disposed of when the using statement goes out of scope. Unless I am missing something the value of rapid disposal (and faster garbage collection) of a resource outweighs the small savings of not having to instantiate the Open/Save File dialogs when they are needed. Even if the open/save dialogs were used every time their parent dialog is used, I don’t see any great value to having them around all the time. My question is am I missing something? Is there some greater efficiency to keeping the open/save dialogs around all the time? I assume the answer will be the same for all the other standard dialogs: ColorDialog, FolderBrowserDialog and FontDialog.

+2  A: 

I think the "on form" option is just supposed to be easier for a novice to implement, since the work is mostly in the editor instead of in code.

Jon B
A: 

The advantage is you can set up the dialog however you need (multiselect, initial directory, extensions) etc in the form designer.

Also the form designer makes it so easy to drag an OpenFileDialog onto your form that many developers just do it. Can't really blame them.

There's no significant advantage or disadvantage to either approach, I doubt you'll find any perceivable performance difference.

Matt Greer
A: 

I generally create form-scope dialogs when I want to persist certain settings, such as filters, without having to set them each place I use the dialog. If a dialog is used only in one place, I use a using statement, as you have above.

NickAldwin
A: 

I always wondered about this approach as well as I prefer the usage pattern in your snippet. The only possible reason I've ever been able to come up with is that by making them components, their properties can be configured in the designer.

Not a very compelling reason (IMHO).

dkackman
A: 

I generally agree. But there could be a slight performance gain if the dialogs have a lot of 'default' property data. Think about a file open dialog with 50 file type filters.

Assaf
+4  A: 

Not necessary. These dialogs have a Dispose() method only because they inherit one from the Component class. They don't override the method since they don't actually have anything to dispose. All the system resources they use get released when the dialog closes. The Component.Dispose() implementation doesn't do any disposing either, it just makes sure that the component is removed from a IContainer collection.

This is otherwise an implementation detail of these dialog wrappers. Other components do normally have use for Dispose(), ToolTip being a good example. The auto-generated Dispose method for a form makes sure that this happens, check out the "components" field in the form's Designer.cs source code file. Which is why Component has a Dispose method that doesn't do anything important by default.

There's one teeny advantage to avoiding dropping the component on the form, it allows the .NET wrapper class for the dialog to be garbage collected. But the kilobyte or so that saves is a micro-optimization.

Take this at face value: if early disposing these class objects would have been important, Microsoft wouldn't have made them components.

Hans Passant
A: 

Good practice would be to do what your doing since you've stated that these dialogs don't need to sit around in memory very long, but if they were to be used many times, it's a good idea to leave it around and dispose of it properly when the application closes, simply depends on the situation.

Given the situation it sounds like your good going either way and I wouldn't lose much sleep over it, but regardless whether it does anything useful in it's dispose method, the way your using it is recommended, its just good practice.

Capital G