tags:

views:

114

answers:

7

Will there be a huge performance difference between:

if (this.chkSelectAll.Checked)
    for (int i = 0; i < this.listBoxColumns.Items.Count; i++)
        this.listBoxColumns.SetSelected(i, true);
else
    for (int i = 0; i < this.listBoxColumns.Items.Count; i++)
        this.listBoxColumns.SetSelected(i, false);

vs.

for (int i = 0; i < this.listBoxColumns.Items.Count; i++)
    this.listBoxColumns.SetSelected(i, this.chkSelectAll.Checked);

Which one is advisable. Concise coding vs. performance gain?

+1  A: 

You have an extra boolean check in the first example. But having said that, I can't imagine that the performance difference will be anything other than negligible. Have you tried measuring this in your particular scenario ?

The second example is preferable since you're not repeating the loop code.

Brian Agnew
+10  A: 

I wouldn't expect to see much performance difference, and I'd certainly go with the latter as it's more readable. (I'd put braces round it though.)

It's quite easy to imagine a situation where you might need to change the loop, and with the first example you might accidentally only change one of them instead of both. If you really want to avoid calling the Checked property in every iteration, you could always do:

bool checked = this.chkSelectAll.Checked;
for (int i = 0; i < this.listBoxColumns.Items.Count; i++)
{
    this.listBoxColumns.SetSelected(i, checked);
}

As ever, write the most readable code first, and measure/profile any performance differences before bending your design/code out of shape for the sake of performance.

Jon Skeet
A: 

Why not use System.Diagnostics.StopWatch and compare the two yourself? However, I don't believe there's going to be any real performance difference. The first example might be faster because you're only accessing chkSelectAll.Checked once. Both are easily readable though.

Jean Azzopardi
+1  A: 

I can't see there being a significant performance difference between the two. The way to confirm it would be to set up a benchmark and time the different algorithms over 1000s of iterations.

However as it's UI code any performance gain is pretty meaningless as you are going to be waiting for the user to read the dialog and decide what to do next.

Personally I'd go for the second approach every time. You've only got one loop to maintain, and the code is clearer.

ChrisF
+1  A: 

Any performance difference will be negligible.

Your primary concern should be code readability and maintainability.

Micro-optimisations such as this are more often than not, misplaced. Always profile before being concerned with performance.

Mitch Wheat
+1  A: 

It's most likely to be negligible. More importantly, however, I feel the need to quote the following:

"Premature optimisation is the root of all evil"

The second is easily the more readable, so simply go with that, unless you later find a need to optimise (which is quite unlikely in my opinion).

Noldorin
+2  A: 

I suppose the performance difference will be barely noticeable. However here's a variation that is both efficient and highly readable:

bool isChecked = this.chkSelectAll.Checked;
for (int i = 0; i < this.listBoxColumns.Items.Count; i++) {
    this.listBoxColumns.SetSelected(i, isChecked);
}

If you're after some real optimization you will also want to pay attention to whether the overhead of accessing "this.listBoxColumns" twice on each iteration is present in the first place and is worth paying attention to. That's what profiling is for.

sharptooth