1

I often see this kind of piece of code as an example of aggregating object enumerables in c#:

IEnumerable<MyCoolObject> myObjects = new List<MyCoolObject>()
{new MyCoolObject(){Value1=1, Value2=10}, new MyCoolObject(){Value1=2, Value2=20}};
MyCoolObject aggregatedObject = myObjects.Aggregate(new MyCoolObject(), (accumlator, next) => new MyCoolObject()
{ 
    Value1=accumlator.Value1 + next.Value1, 
    Value2=accumlator.Value2 + next.Value2
}

My problem with this approach is it creates a new MyCoolObject every iteration, wich seems like a huge waste.

The other common example is this:

MyCoolObject aggregatedObject = new MyCoolObject()
{
    Value1=myObjects.Sum(x=>x.Value1),
    Value2=myObjects.Sum(x=>x.Value2)
}

This one iterates my collection twice, also a big waste, espetially, if there are more fields to aggregate on my objects.

What I figured is, I could do this:

MyCoolObject aggregatedObject = myObjects.Aggregate(new MyCoolObject(), (accumlator, next) =>
{ 
    accumlator.Value1 += next.Value1;
    accumlator.Value2 += next.Value2;
    return accumlator;
};

This one creates a single accumlator object, works on it, and returns it, when finished. To me this looks to be on par with a manual foreach loop performance-wise. I'm surprised, that I don't see this solution often. Are there any problems this solution can introduce, that could explain this?

Marcin Zablocki
  • 10,171
  • 1
  • 37
  • 47
Robert
  • 1,658
  • 16
  • 26
  • 2
    1) The examples you posted are *not* common. 2) Your code *also creates* one temporary object for each input row. Is it a typo? 3) The common usage is to add the current value to the accumulator and return the accumulator – Panagiotis Kanavos Aug 06 '15 at 14:28
  • 1
    @PanagiotisKanavos I suspect that the object creation in his suggested code is just a typo. Other than that, I agree with you. :) – Venemo Aug 06 '15 at 14:38
  • 1
    Indeed, the new object creation was a typo. Never copypaste your bad example. – Robert Aug 06 '15 at 14:42

1 Answers1

2

The common usage is to modify the accumulator and then return it, not to create a new object. In this case, I'd expect to see the following code:

var aggregatedObject = myObjects.Aggregate(new MyCoolObject(),
    (accumulator, next) => {
       accumulator.Value1 +=next.Value1;
       accumulator.Value2 +=next.Value2;
       return accumulator;
    });

Returning a new object from the accumulating function makes sense only if modifying the accumulator has side effects. A good accumulator object though should not produce side effects. In such a case it would be better to use a different object type as the accumulator.

Panagiotis Kanavos
  • 120,703
  • 13
  • 188
  • 236