1

I recently started a new job and have taken responsibility of an already existing project in order to optimize the code and make it more efficient (e.g. eliminating redundant code, moving long processes off the UI thread, etc.).

I recently came across a method which uses a nested foreach statement in order to compare a list against itself to check for duplicate data. However, it uses the ToList() method directly in the foreach statement. For example:

foreach(ObjectRow Outer_Row in t.ToList()) {
   bool bComparing = false;
   foreach(ObjectRow Inner_Row in t.ToList()) {
      //code for comparing
   }
}

I know that this code is generating the list at least twice; once for the outer loop and one for the inner. My question is, is the t.ToList() method actually running for each iteration of the loops? That is, if there are 100 items in t, is ToList() being called a total of 200 times (100 times for the outer loop and 100 times for the inner loop) or is it still only being called twice (once for the outer loop and once for the inner loop)?

I'm thinking that it would be more efficient to do something like the following:

var list = t.ToList();
foreach(ObjectRow Outer_Row in list) {
   bool bComparing = false;
   foreach(ObjectRow Inner_Row in list) {
   //code for comparing
   }
}

It should be noted that t is being generated from a LINQ expression, which results in t being an IEnumerable<ObjectRow>. So would it be even better to simple do this:

foreach(ObjectRow Outer_Row in t) {
   bool bComparing = false;
   foreach(ObjectRow Inner_Row in t) {
      //code for comparing
   }
}

Is there any benefit to enumerating over the list vs. the original IEnumerable?

Thanks in advance for any information and/or suggestions.

Dominick
  • 322
  • 1
  • 3
  • 16
  • 4
    `being called a total of 200 times` 101 times. Once for the outer loop, 100 for the inner, – mjwills Sep 10 '19 at 13:24
  • Your idea would generally be faster, although it is hard to know for sure without seeing the `//code for comparing`. – mjwills Sep 10 '19 at 13:25
  • If what you want is to find duplicates you can order both `IEnumerable`s (separately) and then move like in a merge (of merge sort). – dcg Sep 10 '19 at 13:28
  • 3
    A `ToList()` might be used to create a copy, so the collection is able to be modified within the list. – Jeroen van Langen Sep 10 '19 at 13:29
  • @JeroenvanLangen The lists aren't actually modified at all in the code, they are just looped through. – Dominick Sep 10 '19 at 14:31
  • @mjwills True...the 101 makes more sense now when I think about it. The actual comparing method is quite complicated, and requires WCF calls to a middle tier, so at first I'm just trying to reduce all these nested loops and, in this case, regeneration of lists, when they are not needed. Thanks. – Dominick Sep 10 '19 at 14:33

0 Answers0