foreach in list or foreach in list.where

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

    I think the second is helpful when you edit item in list such as add or remove. If you update information, I think it's not

    Have you profiled it to see which is faster? Or are you asking conceptually?

    There isn't any difference in performance, so use the one you think is easier to read.

    Did you consider case when someone maintaining your code erroneously inserts `list=null` in between declaration and foreach in second case? Sharing your research helps everyone. Tell us what you've tried and why it didn’t meet your needs. This demonstrates that you’ve taken the time to try to help yourself, it saves us from reiterating obvious answers, and most of all it helps you get a more specific and relevant answer. Also see [ask]

    @gnat: thanks for comment, forget list=null, in my example, list always have value

    it's only in your example here and, believe me, only in your imagination. In a real world, after your code is out and given to someone else to maintain, you better expect (and try to prevent) weirdest errors from them, see Coding For Violent Psychopaths

  • david.pfx

    david.pfx Correct answer

    6 years ago

    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.

  • 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.

License under CC-BY-SA with attribution


Content dated before 6/26/2020 9:53 AM