CODEWITHSUNDEEP
c#linq
Mrinal Kamboj
Mrinal Kamboj

Reputation: 11478

Converting foreach to Linq

Current Code:

For each element in the MapEntryTable, check the properties IsDisplayedColumn and IsReturnColumn and if they are true then add the element to another set of lists, its running time would be O(n), there would be many elements with both properties as false, so will not get added to any of the lists in the loop. 

 foreach (var mapEntry in MapEntryTable)
 {
   if (mapEntry.IsDisplayedColumn)
      Type1.DisplayColumnId.Add(mapEntry.OutputColumnId);

   if (mapEntry.IsReturnColumn)
      Type1.ReturnColumnId.Add(mapEntry.OutputColumnId);
 }

Following is the Linq version of doing the same:

MapEntryTable.Where(x => x.IsDisplayedColumn == true).ToList().ForEach(mapEntry => Type1.DisplayColumnId.Add(mapEntry.OutputColumnId));       

MapEntryTable.Where(x => x.IsReturnColumn == true).ToList().ForEach(mapEntry => Type1.ReturnColumnId.Add(mapEntry.OutputColumnId));

I am converting all such foreach code to linq, as I am learning it, but my question is:

UPDATE:

Consider the condition where out of 1000 elements in the list 80% have both properties false, then does where provides me a benefit of quickly finding elements with a given condition.

Type1 is a custom type with set of List<int> structures, DisplayColumnId and ReturnColumnId

Upvotes: 2

Views: 664

Answers (5)

Stephen Kennedy
Stephen Kennedy

Reputation: 21568

Your LINQ isn't quite right as you're converting the results of Where to a List and then pseudo-iterating over those results with ForEach to add to another list. Use ToList or AddRange for converting or adding sequences to lists.

Example, where overwriting list1 (if it were actually a List<T>):

list1 = MapEntryTable.Where(x => x.IsDisplayedColumn == true)
.Select(mapEntry => mapEntry.OutputColumnId).ToList();

or to append:

list1.AddRange(MapEntryTable.Where(x => x.IsDisplayedColumn == true)
.Select(mapEntry => mapEntry.OutputColumnId));

Upvotes: 1

Ryan Mann
Ryan Mann

Reputation: 5357

The performance of foreach vs Linq ForEach are almost exactly the same, within nano seconds of each other. Assuming you have the same internal logic in the loop in both versions when testing.

However a for loop, outperforms both by a LARGE margin. for(int i; i < count; ++i) is much faster than both. Because a for loop doesn't rely on an IEnumerable implementation (overhead). The for loop compiles to x86 register index/jump code. It maintains an incrementor, and then it's up to you to retrieve the item by it's index in the loop.

Using a Linq ForEach loop though does have a big disadvantage. You cannot break out of the loop. If you need to do that you have to maintain a boolean like "breakLoop = false", set it to true, and have each recursive exit if breakLoop is true... Bad performing there. Secondly you cannot use continue, instead you use "return".

I never use Linq's foreach loop.

If you are dealing with linq, e.g.

List<Thing> things = .....;
var oldThings = things.Where(p.DateTime.Year < DateTime.Now.Year);

That internally will foreach with linq and give you back only the items with a year less than the current year. Cool..

But if I am doing this:

List<Thing> things = new List<Thing>();

foreach(XElement node in Results) {
    things.Add(new Thing(node));
}

I don't need to use a linq for each loop. Even if I did...

foreach(var node in thingNodes.Where(p => p.NodeType == "Thing") {
    if (node.Ignore) {
        continue;
    }
    thing.Add(node);
}

even though I could write that cleaner like

foreach(var node in thingNodes.Where(p => p.NodeType == "Thing" && !node.Ignore) {
    thing.Add(node);
}

There is no real reason I can think of to do this..>

   things.ForEach(thing => {
       //do something
       //can't break
       //can't continue
       return; //<- continue
   });

And if I want the fastest loop possible,

for (int i = 0; i < things.Count; ++i) {
    var thing = things[i];
    //do something
}

Will be faster.

Upvotes: 1

maraaaaaaaa
maraaaaaaaa

Reputation: 8193

I would say stick with the original way with the foreach loop, since you are only iterating through the list 1 time over.

also your linq should look more like this:

list1.DisplayColumnId.AddRange(MapEntryTable.Where(x => x.IsDisplayedColumn).Select(mapEntry => mapEntry.OutputColumnId));       

list2.ReturnColumnId.AddRange(MapEntryTable.Where(x => x.IsReturnColumn).Select(mapEntry => mapEntry.OutputColumnId));

Upvotes: 1

Servy
Servy

Reputation: 203818

ForEach ins't a LINQ method. It's a method of List. And not only is it not a part of LINQ, it's very much against the very values and patterns of LINQ. Eric Lippet explains this in a blog post that was written when he was a principle developer on the C# compiler team.

Your "LINQ" approach also:

  • Completely unnecessarily copies all of the items to be added into a list, which is both wasteful in time and memory and also conflicts with LINQ's goals of deferred execution when executing queries.
  • Isn't actually a query with the exception of the Where operator. You're acting on the items in the query, rather than performing a query. LINQ is a querying tool, not a tool for manipulating data sets.
  • You're iterating the source sequence twice. This may or may not be a problem, depending on what the source sequence actually is and what the costs of iterating it are.

A solution that uses LINQ as much as is it is designed for would be to use it like so:

foreach (var mapEntry in MapEntryTable.Where(entry => mapEntry.IsDisplayedColumn))
    list1.DisplayColumnId.Add(mapEntry.OutputColumnId);
foreach (var mapEntry in MapEntryTable.Where(entry => mapEntry.IsReturnColumn))
    list2.ReturnColumnId.Add(mapEntry.OutputColumnId);

Upvotes: 7

moarboilerplate
moarboilerplate

Reputation: 1643

In C#, to do what you want functionally in one call, you have to write your own partition method. If you are open to using F#, you can use List.Partition<'T>

https://msdn.microsoft.com/en-us/library/ee353782.aspx

Upvotes: 0

Related Questions