LINQ expression to optimize syntax?

Question

foreach (var item in mainCanvas.Children)
{
    if (item is Button)
    {
        (item as Button).Content = "this is a button";
    }                
}

Can I use LINQ or other feature of .NET 4 to be more concise (maybe performant)?

Please read item 3 of effective C#. How about this: Button castedItem = null; foreach ... castedItem = item as Button; if (castedItem != null) { // do work here } } // end foreach. That way you are cutting the use of reflection into 2, this gaining some speed-up. A LINQ query should be as performant, probably more readable. — Hamish Grubijan
– Hamish Grubijan, Commented Sep 30, 2010 at 16:16
Concise and performant are sometimes at odds. If this is a deep inner loop, profiling indicates you have a performance problem here and the collection under consideration is an array, you can get better performance by indexing with a standard for loop. — Dan Bryant
– Dan Bryant, Commented Sep 30, 2010 at 17:09
@Dan Bryant, see Item 11 of "Effective C#" titled "Prefer foreach loops". The JIT compiler is not dumb and does not need help. This was true back in .Net 1.1, so it cannot be less true now. I would be surprised ... if for ever beats non-stupidly constructed foreach in C#. — Hamish Grubijan
– Hamish Grubijan, Commented Sep 30, 2010 at 17:33
@Hamish, I encountered a specific case where this was beneficial (specific target was .NET 3.5 for Silverlight), but the improvement was only noticeable because the inner loop was being invoked millions of times. I definitely agree that it's best to prefer foreach and LINQ. — Dan Bryant
– Dan Bryant, Commented Sep 30, 2010 at 18:20
@Dan Bryant, do you think you could paste an example of what the code looked like? I am quite curious now. — Hamish Grubijan
– Hamish Grubijan, Commented Sep 30, 2010 at 19:26

Mark Byers · Accepted Answer · 2010-09-30 17:41:35Z

12

You can use Enumerable.OfType:

foreach (var button in mainCanvas.Children.OfType<Button>())
{
    button.Content = "this is a button";
}

Performance Measurements

Method 1: OPs original suggestion

foreach (var item in mainCanvas.Children)
{
    if (item is Button)
    {
        (item as Button).Content = "this is a button";
    }                
}

Method 2: OfType

foreach (var button in mainCanvas.Children.OfType<Button>())
{
    button.Content = "this is a button";
}

Method 3: Only cast once

foreach (var item in mainCanvas.Children)
{
    Button button = item as Button;
    if (button != null)
    {
        button.Content = "this is a button";
    }                
}

Method 4: for loop:

List<object> children = mainCanvas.Children;
for (int i = 0; i < children.Count; ++i)
{
    object item = children[i];
    if (item is Button)
    {
        (item as Button).Content = "this is a button";
    }                
}

Results

Iterations per second

Method 1: 18539180
Method 2:  7376857
Method 3: 19280965
Method 4: 20739241

Conclusion

The biggest improvement can be gained by using a simple for loop instead of foreach.
It is also possible to improve performance slightly by casting only once.
Using OfType is considerably slower.

But remember to optimize readability first, and only optimize performance if you have performance profiled and found that this specific code is the performance bottleneck.

edited Sep 30, 2010 at 17:41

answered Sep 30, 2010 at 16:10

Mark Byers

844k202 gold badges1.6k silver badges1.5k bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Hamish Grubijan Over a year ago

Looks like a winner to me - short and fast. Doing stuff with lambdas seems like an overkill.

serhio Over a year ago

@Mark Byers: Buttons was for the example purpose. In application I have a graph with a huge number of curves. I select them and move...

danijels Over a year ago

I think performance-wise IEnumerable<T>.OfType() will do just as fine as original post. So if you want concise, there is a one-liner solution here, if performance is critical, I believe for-loop performs better than foreach and List.ForEach(), but that is marginal.

Mark Byers Over a year ago

@serhio: The best way to find out which is fastest is to measure it. I've updated my answer with some measurements I've made.

Hamish Grubijan Over a year ago

@Mark, what about the 5th method - the hybrid of method 3 and 4? How much better is it to "cast once"? Also, this might be a dumb idea, but how about the 6th method - using object instead of var in ... say method 3? What if something like IButton is used instead of Button when casting? Finally, how do you measure the performance? Are the numbers consistent from run to run? With JIT, pre-emption, noise is possible. Thanks.

danijels · Accepted Answer · 2010-09-30 16:11:14Z

3

One line should do it

mainCanvas.Children.OfType<Button>.ToList().ForEach(b => b.Content = "this is a button");

answered Sep 30, 2010 at 16:11

danijels

5,3214 gold badges30 silver badges37 bronze badges

5 Comments

danijels Over a year ago

That is always debatable, but this is just to answer the question, which was, after all, about it being concise. Everyone should take whatever flavor suits them.

Hamish Grubijan Over a year ago

Turning an iterator into a List, and then iterating over it is expensive.

danijels Over a year ago

That depends on the contents of .Children. If there is hundreds of items and only 3-4 buttons, this should be a good way. Can't tell here.

serhio Over a year ago

@hamish-grubijan: foreach should do the same thing, isn't it?

Hamish Grubijan Over a year ago

@serhio, foreach should operate on an IEnumerable, which can be a lazy collection.

Cheng Chen · Accepted Answer · 2010-09-30 16:10:12Z

1

See if there is an OfType<T> extension.

foreach (var item in mainCanvas.Children.OfType<Button>()) 
{ 
    item.Content = "this is a button"; 
}

If not, you can use:

foreach (var item in mainCanvas.Children.Where(item=>item is Button).Cast<Button>()) 
{ 
    item.Content = "this is a button"; 
}

answered Sep 30, 2010 at 16:10

Cheng Chen

43.7k18 gold badges118 silver badges184 bronze badges

1 Comment

Bear Monkey Over a year ago

If there is not an OfType extension there will also not be a Where or a Cast extension. All 3 methods have existed since the first version of Enumerable class. Also Where cannot be used UIElementCollection as it not generic.

Judah Gabriel Himango · Accepted Answer · 2010-09-30 17:45:08Z

1

Not that it's particularly superior, but there is something nice about this syntax:

Using LINQ and the Microsoft ReactiveExtensions framework,

mainCanvas.Children
   .OfType<Button>()
   .Do(b => b.Content = "I'm a button!")
   .Run();

answered Sep 30, 2010 at 17:45

Judah Gabriel Himango

60.2k39 gold badges162 silver badges218 bronze badges

Comments

csharpstudent · Accepted Answer · 2012-07-09 16:09:07Z

1

To only iterate over actual buttons in a collection, you can do:

foreach(Button button in mainCanvas.Children)
     button.Content = "this is a button";

As far as I understand, this is syntax sugar that gets converted into Method 4 above. But don't quote me on that. (EDIT: Method 3 I meant)

edited Jul 9, 2012 at 16:09

answered Jul 6, 2012 at 23:54

csharpstudent

112 bronze badges

Collectives™ on Stack Overflow

LINQ expression to optimize syntax?

5 Answers 5

5 Comments

5 Comments

1 Comment

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

5 Comments

5 Comments

1 Comment

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related