1

I don't know what to call this question.

This is my example:

foreach (var item in lstItem.Where(item => stock.ItemCode == item.ItemCode))
{
     stock.ItemName = item.ItemName;
     stock.ItemUnitName = item.ItemUnitName;
     stock.BrandName = item.BrandName;
     stock.FamilyName = item.FamilyName;
}

or

// Assume list has items
var list = lstItem.Where(item => stock.ItemCode == item.ItemCode);
foreach (var item in list)
{
     stock.ItemName = item.ItemName;
     stock.ItemUnitName = item.ItemUnitName;
     stock.BrandName = item.BrandName;
     stock.FamilyName = item.FamilyName;
}

Which one is better?

marked as duplicate by gnat, Bart van Ingen Schenau, user40980, GlenH7, amon May 8 '14 at 17:05

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

0

There are two interesting questions wrapped into one here.

First, is it better to break up more complex expressions into simpler ones, and perhaps take the opportunity to give the variables helpful names?

This is often a really good idea, but in this case the code might better read as:

var wanted_items = lstItem.Where(item => stock.ItemCode == item.ItemCode);
foreach (var item in wanted_items)

Second, are there performance issues in breaking up expressions like this?

Not so easy to answer, in the general case. In most languages a construct of this kind would lead to all items of the list being processed to filter out those required, and then individual items processed again to apply the assignment. If there is only one match then the first pass will (on average) examine twice as many items as needed.

But this is C# and Linq, so now the list will be either an iterator or it will be translated into SQL, both of which optimise processing, to a greater or lesser extent. In this particular case the performance will be (should be) exactly the same whether the expression is split or not, but there are closely related constructs in which splitting the expression like this can crystallise a particular path for execution and have quite severe performance impact.

Ultimately, you should always write the most readable code you can using the best possible algorithms and best possible use of underlying framework libraries. If you then have concerns for performance, you need to benchmark and measure before even thinking about changing the code.

  • Thank you for your responding, I get the first example many times in my company. But I need a benchmark to make sure anything. – Steve Lam May 8 '14 at 7:03
1

Both of your examples are functionally equivalent.

The reason for this is that an enumeration is not evaluated until it is absolutely needed. Looking at your second example:

var list = lstItem.Where(item => stock.ItemCode == item.ItemCode);
foreach (var item in list)
{
 stock.ItemName = item.ItemName;
 stock.ItemUnitName = item.ItemUnitName;
 stock.BrandName = item.BrandName;
 stock.FamilyName = item.FamilyName;
}

What happens is that list is not a list. Rather it is an enumerator of lstItem with the Where condition on it. That enumerator gets evaluated once for each iteration of the foreach loop.

The reason for this is that this will allow for maximum performance if the foreach loop is exited early. The enumerator might have to do some complex calculations when it is run and so short circuiting out can, in some cases, result in much faster code.

It's best not to think of var list = lstItem.Where(item => stock.ItemCode == item.ItemCode); as giving you a list. Rather, it's giving you an object which will return the next item when it's asked until it runs out of items. Theoretically it could go on forever and it's entirely possible to write an enumerator that cycles around on itself.

Personally, I prefer your first code snippet as it requires less reading. Note that when the compiler optimises your code, the temporary list variable will get optimised away unless it's used later on.

Not the answer you're looking for? Browse other questions tagged or ask your own question.