CODEWITHSUNDEEP
c#winforms
recursive
recursive

Reputation: 86084

Any overhead to redundantly setting Control properties?

I'm dealing with a lot of code that looks like this:

 if (btnLeftDock.BackColor != SystemColors.ButtonFace)
 {
    btnLeftDock.BackColor = SystemColors.ButtonFace;
 }
 if (btnRightDock.BackColor != SystemColors.ButtonFace)
 {
    btnRightDock.BackColor = SystemColors.ButtonFace;
 }
 if (btnTopDock.BackColor != SystemColors.ButtonFace)
 {
    btnTopDock.BackColor = SystemColors.ButtonFace;
 }
 if (btnBottomDock.BackColor != SystemColors.ButtonFace)
 {
    btnBottomDock.BackColor = SystemColors.ButtonFace;
 }

The only reason I can imagine to do this is that there is theoretically some winforms-specific overhead to setting control colors like this:

 btnLeftDock.BackColor = SystemColors.ButtonFace;
 btnRightDock.BackColor = SystemColors.ButtonFace;
 btnTopDock.BackColor = SystemColors.ButtonFace;
 btnBottomDock.BackColor = SystemColors.ButtonFace;

I think it's much easier to read, and I can't see any performance difference, but the original developer must have had some justification. (right?)

Upvotes: 2

Views: 234

Answers (4)

Hans Passant
Hans Passant

Reputation: 941675

There is something special about the BackColor property, it is an ambient property. What that means is that if the property was never assigned then the BackColor of the control will be the same as the parent's BackColor value.

That's highly desirable, it provides automatic consistent background color values. If the parent changes its BackColor then all child controls change it too, to the same value. As long as they never assigned it themselves.

That might have paralyzed the original author a bit. But since he used system colors, the test shouldn't be necessary. I think.

Upvotes: 1

Ani
Ani

Reputation: 113442

Looking at the code from reflector, there does appear to be some minor performance benefit. Nothing of significance though; so I personally wouldn't bother with the check unless the redunant set was identified as a bottleneck. In particular, the OnBackColorChanged handler will not execute on a redundant set.

public override Color BackColor
{
    set
    {
        if (base.DesignMode)
        {
            if (value != Color.Empty)
            {
                PropertyDescriptor descriptor = TypeDescriptor.GetProperties(this)["UseVisualStyleBackColor"];
                if (descriptor != null)
                {
                    descriptor.SetValue(this, false);
                }
            }
        }
        else
        {
            this.UseVisualStyleBackColor = false;
        }
        base.BackColor = value;
    }
}

where base.BackColor, defined on System.Windows.Forms.Control is:

public virtual Color BackColor
{
    set
    {
        if ((!value.Equals(Color.Empty) && !this.GetStyle(ControlStyles.SupportsTransparentBackColor)) && (value.A < 0xff))
        {
            throw new ArgumentException(SR.GetString("TransparentBackColorNotAllowed"));
        }
        Color backColor = this.BackColor;
        if (!value.IsEmpty || this.Properties.ContainsObject(PropBackColor))
        {
            this.Properties.SetColor(PropBackColor, value);
        }
        if (!backColor.Equals(this.BackColor))
        {
            this.OnBackColorChanged(EventArgs.Empty);
        }
    }
}

Upvotes: 1

Rob Fonseca-Ensor
Rob Fonseca-Ensor

Reputation: 15621

These are buttons, right?

You should find that BackColorChanged is not being fired, so I can't imagine any functional side effects that the original developer is trying to avoid.

Push it to prod :)

Upvotes: 1

Matt Greer
Matt Greer

Reputation: 62037

I would suspect setting a visual property like BackColor will cause the control to get invalidated as the reason for doing this (although still a bit misguided). So an even better solution is to just sandwich these changes between BeginUpdate() and EndUpdate() calls, which will cause one invalidation no matter what you do.

Upvotes: 0

Related Questions