Any overhead to redundantly setting Control properties?

Question

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?)

Answer 1

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.

Answer 2

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 :)

Answer 3

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

Answer 4

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.