Is using linq in this situation overkill

Question

I am thinking about a re-factor, but I don't know if the end result is just overkill. Currently I have

    IList<myobjecttypebase> m_items;

   public int GetIncrementTotal()
   {
     int incrementTot;
     foreach(myobjecttypebase x in m_items)
     { incrementTot += x.GetIncrement(); }
   }

Would it be overkill and/or less efficient to use linq for the ForEach

m_items.ToList().ForEach(x => incrementTot += x.GetIncrement());

Would the cast be a significant overhead here?

Answer 1

The overhead will be in iterating the collection twice.

m_items.ToList() // First iteration to add to the list all the items
   .ForEach(x => incrementTot += x.GetIncrement()); /* Second iteration to
                                                       perform logic */

ToList doesn't perform the iteration in lazy manner as most LINQ statements, therefor it will force the code iterate over the collection twice.

In general the LINQ form seems nicer to read, but if you're worrying about performance you better avoid it.

Answer 2

Why not use the SUM operator directly on the IList?

It does have several overloads using Func delegates:

m_items.Sum(x => x.GetIncrement());

Answer 3

Don't do it this way.
The ToList() here will allocate and populate a new list, IMHO not necessary. While it's a trivial exercise to write a ForEach extension method to iterate over any object that implements IEnumberable, I'd advise against it (see this article by Eric Lippert foreach V's ForEach).

I'd go with the foreach.

P.S. you need to initialise incrementTot (sorry, I couldn't help myself)

Answer 4

As Oded mentioned use SUM

incrementTot = m_items.ToList().Sum(x => x.GetIncrement());

Answer 5

ForEach isn't part of LINQ - it's part of the List<T> API and has been since .NET 2.0.

The reason it's not part of LINQ is that it's a naturally side-effecting method... and LINQ doesn't encourage side-effects.

Using Sum is the right solution here, but you don't need to create a list first:

return m_items.Sum(x => x.GetIncrement());

Answer 6

The ToList method is an extension method used with LINQ, but the ForEach method is just a method in the List class.

The major overhead here is the call to the ToList method, which creates a new List from the collection. The ForEach also has a slight overhead as it has to call a delegate for each item.

If you want to use LINQ methods, the Aggregate method seems more appropriate:

public int GetIncrementTotal() {
  return m_items.Aggregate(0, (t, i) => t + i.GetIncrement());
}

Or Sum:

public int GetIncrementTotal() {
  return m_items.Sum(i => i.GetIncrement());
}

Either has a slight overhead over your original version, so if you want the most efficient, just stick to a simple loop.