C# performance and readability with buttons enabling/disabling

Question

Hey all,

I've often found myself in front of that kind of code :

if(Something > 0)
{
    btnOne.Enabled = true;
    btnTwo.Enabled = true;
    btnThree.Enabled = false:
}
else
{
    btnOne.Enabled = false;
    btnTwo.Enabled = false;
    btnThree.Enabled = true:
}

And I've always wondered if it was better to let it like that, or put it like this :

bool ButtonEnabled = (Something > 0);

btnOne.Enabled = ButtonEnabled;
btnTwo.Enabled = ButtonEnabled;
btnThree.Enabled = !ButtonEnabled;

Realizing the question is a bit argumentative, let's put aside the "readability" factor and concentrate on the performance factor... What would be best ? One more assignation or a condition ?

Thanks in advance for your advices (or a even better way to write it) !

Edit : Corrected an error in my second snippet. Edit : The two initial examples weren't equivalent...

Answer 1

Whatever performance difference you may see between the two will most likely be insignificant in this case, so I would pick the one that is most readable.

Answer 2

That depends on the properties being called. As you know, a property can do any amount if things. In Windows Forms or WPF, I wouldn't worry about it. I'd argue for the latter style for correctness and readability. If you set all necessary variables every time, there is less chance of missing something and leaving one button in an invalid state.

I'd do something like

bool ButtonEnabled = (Something > 0);
btnOne.Enabled = ButtonEnabled;
btnTwo.Enabled = ButtonEnabled;
btnThree.Enabled = !ButtonEnabled;
btnFour.Enabled = !ButtonEnabled;

Answer 3

You can't compare the two pieces of code, neither on readability nor on performance, as they give different results.

The version of the first code that is equivalent to the second would be:

if(Something > 0)
{
    btnOne.Enabled = true;
    btnTwo.Enabled = true;
    btnThree.Enabled = false;
    btnFour.Enabled = false;
}
else
{
    btnOne.Enabled = false;
    btnTwo.Enabled = false;
    btnThree.Enabled = true;
    btnFour.Enabled = true;
}

The version of the second code that is equivalent to the first would be:

bool ButtonEnabled = (Something > 0);

btnOne.Enabled = ButtonEnabled ? true : btnOne.Enabled;
btnTwo.Enabled = ButtonEnabled ? true : btnTwo.Enabled;
btnThree.Enabled = !ButtonEnabled ? false : btnThree.Enabled;
btnFour.Enabled = !ButtonEnabled ? false : btnFour.Enabled;

So, the first piece of code is clearly more efficient and readable than it's equivalent alternative, and the second piece of code is shorter and somewhat easier to maintain than it's equivalent alternative.

Answer 4

Yep, unlike your application has one hundred thousand buttons displayed at the same time, concentrate HEAVILY on readability, not on micro-optimization ! The time it will take the UI layer to update the visual of your control will be 10.000 times longer than your "Enabled" assignments anyway !

Solution 2 is actually nearly what you will want to do when using data-binding (you were very near :p). Actually, you would code something more like:

public class MyClass {
    public bool IsSomethingTrue { get; set; } // with notification on property changed
    public bool IsSomethingFalse { get { return !IsSomethingTrue; } }

    private AMethod() {
        ...
        IsSomethingTrue = Something > 0;
        ...
    }

And your UI would be something like (WPF flavored):

<Button IsEnabled={Binding IsSomethingTrue} /> <!-- btn 1 -->
<Button IsEnabled={Binding IsSomethingTrue} /> <!-- btn 2 -->
<Button IsEnabled={Binding IsSomethingFalse} /> <!-- btn 3 -->
<Button IsEnabled={Binding IsSomethingFalse} /> <!-- btn 4 -->
<!-- Want a 5th button ? just add it without changing your code-behind ! -->

This pattern allows you to add as many buttons as you want without changing your methods each time. This is especially helpful when methods tends to be quite complicated, and it improves the readability.

It works for WPF, Qt, Java, and I think Winforms should provide some data-binding capability.