views:

339

answers:

7

When adding controls to a form at runtime, you can do either of the following:

Button btn = new Button();
//...
this.Controls.Add(btn);

or

Button x = new Button();
//...
btn.Parent = this;

I had assumed that they were the same, and it was just down to personal preference which way to do it, but someone at work mentioned that the second method is worse as the button will not get disposed when the form is disposed (assuming no event handlers have been added and are being held onto).

This didn't make a lot of sense to me so I had a look online, but couldn't find anything to say one way or the other.

Does someone know the answer or can point me in the right direction?

A: 

I think both result to the same thing

Bertvan
+3  A: 

I've always preferred identifying which object's controls I'm going to add the new control to...

Button btn = new Button();
this.PlaceHolder1.Controls.Add(btn);

Button btn2 = new Button();
this.PlaceHolder2.Controls.Add(btn2);

I feel that this is easier to read and you don't have to do any family tree analysis to figure out who the parent is...

I believe that using the .Parent code internally does the .Controls.Add, so they should have the same end result, but to me it comes down to code readability.

There is a similar question here on StackOverflow.

RSolberg
A: 

I personally like

Button btn = new Button();
//...
this.Controls.Add(btn);

Because, it is more explicit and readable code.

Syed Tayyab Ali
+3  A: 

I can see where the button disposal could be an issue if you're writing an application that is opening and closing a lot of forms during its duration. You would need to make sure you have some proper disposal code in hand to make sure the application doesn't suck up too much memory.

That aside, I like the first statement because it more clearly explains what your code is doing. You're creating a new button, and you're adding it to the existing controls on the page. You can read right past this when debugging/refactoring and understand what is going on. In the second group of code, this is slightly more vague. If you brushed over the the initial button declaration and saw the btn.Parent = this statement, you could be led to believe that you were reassigning the button to a new form, or something to that effect.

It does sound a bit nitpicky, but lately I've been helping some co-workers by showing them some of my code and I'm coming to find that while there is definitely more than 1 way to skin a cat, sometimes there a certain way of skinning it that explains itself much better when looking at things in the future.

Dillie-O
+1  A: 

In the second case, the control may not get disposed when the form does (I'm not sure if it does or not), but it should get released by the next round of garbage collection anyway, since there shouldn't be any hard references to it after the disposal of the form. The upshot is, whether the button gets disposed with the form is a non-issue for most applications. In the vast majority of apps that use forms, the user is the bottleneck, so whether you have to wait for one or two passes of the garbage collector before a collection of form controls is disposed shouldn't impact your design decision.

That having been said, I prefer

this.Controls.Add(btn);

because it seems to be more semantically proper for what you're actually doing. I always use this method rather than setting the Control.Parent property.

Dathan
+9  A: 

Since speculating is a waste of time, I grabbed my copy of Reflector and had a look at the actual code. The Parent property calls the ParentInternal property, which in turn calls value.Controls.Add(this)

/* this code is part of the .NET Framework was decompiled by Reflector and is copyright Microsoft */
    internal virtual Control ParentInternal
    {
        get
        {
            return this.parent;
        }
        set
        {
            if (this.parent != value)
            {
                if (value != null)
                {
                    value.Controls.Add(this);
                }
                else
                {
                    this.parent.Controls.Remove(this);
                }
            }
        }
    }

Based on this code, the methods are equivalent and this is strictly a matter of preference.

Jack Bolding
oh. you beat me to it! nobody should be without the reflector.
PeterAllenWebb
For support purposes, .Parent is far harder to read and interpret than .ControlsAdd();
RSolberg
@RSolberg -- I don't disagree that Controls.Add is more explicit, but this was not the question posed. The question is will there be issues with Dispose using the .Parent pattern. Based on the actual code, there will not be any.
Jack Bolding
A: 

It is truly a matter of taste. Here is what happens when you set the Parent property on a Control. This code comes courtesy of .NET Reflector.

set
{
    if (this.parent != value)
    {
        if (value != null)
        {
            value.Controls.Add(this);
        }
        else
        {
            this.parent.Controls.Remove(this);
        }
    }
}
PeterAllenWebb