Simple LINQ statement for report limit

What would be the most efficent way to calculate this?

var reportLimit = 96*1024;

IEnumerable<int> memoryInUse =
things
.Where(sample => sample.IsOn)
.Select(sample => sample.MemoryInMb)
.ToArray();

int totalUnderReportLimit = memoryInUse.Where(ram => ram <= memoryCalcFactor).Sum();
int totalOverEqualReportLimit = memoryInUse.Count(ram => ram > memoryCalcFactor) * memoryCalcFactor;


• If you want to find out what is faster, running benchmarks is exactly what you should do. I'm not sure why are you avoiding that. Commented Jun 21, 2012 at 23:10
• I agree, the ToArray() seems unnecessary here. Commented Jun 21, 2012 at 23:16

Well here's my take on it. If you want efficient in terms of speed, write a loop. LINQ is not for writing fast code, it's for writing concise code.

memoryInUse is just a collection of the memory counts of all the machines that are on. You then essentially partition those machines by some memoryCalcFactor to do some calculation.

The call to ToArray(), while somewhat helpful (you do the filter/projection once) isn't really needed. You can perform your calculation in one pass and therefore don't need it at all. Look at the following lines in your calculation:

var x = memoryInUse.Where(ram => ram <= memoryCalcFactor).Sum();
var y = memoryInUse.Count(ram => ram > memoryCalcFactor) * memoryCalcFactor;
return x + y;


What are we doing here?

1. Adding up all the sizes that are less than some factor
2. Counting all those that are greater than that factor multiplying by that factor.
3. Adding the previous results

Looking at this at a much higher level, what are we doing here? We're adding up the sizes limiting each size by some factor (a maximum). Write your code to do that.

This is how I'd write it:

var memorySizes = machines
.Where(machine => machine.IsOn)
.Select(machine => machine.MemoryInMB);
var result = memorySizes.Sum(size => Math.Max(size, memoryCalcFactor));


Otherwise if you don't want to take the performance hit LINQ will give you and use a loop, here's the equivalent:

var sum = 0;
foreach (var machine in machines)
{
if (machine.IsOn)
{
sum += Math.Max(machine.MemoryInMB, memoryCalcFactor);
}
}

• A .where(...).aggregate(...) should I would think perform an identical single loop... though the aggregate function can feel a little strange to folks not familiar with it. Commented Jun 25, 2012 at 21:29
• @Jimmy: Yeah, I initially started off using aggregate when thinking about this until I realized we were just adding every number in some form. Commented Jun 25, 2012 at 23:57

I guess one way to potentially speed it up (if the issue was with running over the enumerable twice) would be to use a foreach. However, I don't think it's as readable as your solution;

int countOverLimit = 0;

foreach(var ram in memoryUse)
{
if(ram <= memoryCalcFactor)
{
totalUnderReportLimit += ram;
}
else if(ram > memoryCalcFactor)
{
countOverLimit++;
}
}

totalOverEqualReportLimit = countOverLimit * memoryCalcFactor;


+1 to @Jeff. Staying at low level (if there is not a more effective algorithm) and in languages which does not support LINQ I'd create two methods with single responsibilities (calculateTotalUnderReportLimit and calculateTotalOverEqualReportLimit, for example). If it was a performance bottleneck it still could be optimized but until that it would be easier to read and maintain.