CODEWITHSUNDEEP
c#linq
nkirkes
nkirkes

Reputation: 2665

Cleaning up a simple foreach with linq

The following method is pretty simple, I'm trying to determine a line-item rate by matching up another property of the line-item with a lookup from a parent object. There's a few things I don't like about it and am looking for elegant solutions to either make the method smaller, more efficient, or both. It works in it's current state and it's not like it's noticeably inefficient or anything. This isn't mission critical or anything, more of a curiosity. 

    private decimal CalculateLaborTotal()
    {
        decimal result = 0;

        foreach (ExtraWorkOrderLaborItem laborItem in Labor)
        {
            var rates = (from x in Project.ContractRates where x.ProjectRole.Name == laborItem.ProjectRole.Name select x).ToList();
            if (rates != null && rates.Count() > 0)
            {
                result += laborItem.Hours * rates[0].Rate;
            }
        }
        return result;
    }

I like the idea of using List<T>.ForEach(), but I was having some trouble keeping it succinct enough to still be easy to read/maintain. Any thoughts?

Upvotes: 0

Views: 189

Answers (3)

Greg Beech
Greg Beech

Reputation: 136587

Something like this should do it (untested!):

var result = 
    (from laborItem in Labor
     let rate = (from x in Project.ContractRates 
                 where x.ProjectRole.Name == laborItem.ProjectRole.Name 
                 select x).FirstOrDefault()
     where rate != null
     select laborItem.Hours * rate.Rate).Sum();

Or (assuming only one rate can match) a join would be even neater:

var result = 
    (from laborItem in Labor
     join rate in Project.ContractRates
         on laborItem.ProjectRole.Name equals rate.ProjectRole.Name
     select laborItem.Hours * rate.Rate).Sum();

Upvotes: 4

Jon Skeet
Jon Skeet

Reputation: 1500075

Okay, well how about this:

// Lookup from name to IEnumerable<decimal>, assuming Rate is a decimal
var ratesLookup = Project.ContractRates.ToLookup(x => x.ProjectRole.Name,
                                                 x => x.Rate);

var query = (from laborItem in Labor
             let rate = ratesGroup[laborItem].FirstOrDefault()
             select laborItem.Hours * rate).Sum();

The advantage here is that you don't need to look through a potentially large list of contract rates every time - you build the lookup once. That may not be an issue, of course.

Upvotes: 2

Dario
Dario

Reputation: 49208

Omit the rates != null check - A linq query may be empty but not null. If you just need the first element of a list, use List.First or List.FirstOrDefault.

There is no advantage in using List<T>.ForEach.

Upvotes: 0

Related Questions