Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I was wondering if there's a better way of doing LINQ projections than what I'm doing below. This method is one of many methods creating a view of an object tree.

The first thing that I don't like is that I'm returning an anonymous object. Should I create a BreakdownReport class or something else instead?

Also, the copying of property values from right to left is a bit of a hassle.

public object GetBreakdownsReport()
{
    return CurrentSecurity
        .Breakdowns
        .SelectMany(b => b.BreakdownLines, (b, bdl) => new
        {
            AccountId = b.Account.Id,
            Structure = b.Account.Structure,
            Market = b.Account.Market,
            BeneficiaryId = bdl.Beneficiary.Id,
            BeneficiaryType = bdl.Beneficiary.BeneficiaryType,
            CountryCode = bdl.Beneficiary.CountryCode,
            CountryName = bdl.Beneficiary.CountryName,
            DtaRate = bdl.Beneficiary.DtaRate,
            HoldingCode = b.HoldingCode,
            InstructionAmount = bdl.InstructionAmount,
            NarrativeText = bdl.Narrative.Text,
            Chapter = bdl.Narrative.Chapter,
            Paragraph = bdl.Narrative.Paragraph,
            Subparagraph = bdl.Narrative.Subparagraph,
            TaxRate = bdl.Narrative.TaxRate,
        })
        .ToList();
}
share|improve this question
    
You can't return an anonymous object. So yes, you'll have to create another object type to project it into. –  krillgar Jul 25 at 14:45
    
Actually this does work since I'm return it as an object and use it as source for a datagrid. –  Aetherix Jul 25 at 14:57
    
I'd be tempted to create a Breakdown class and us Automapper. –  DavidG Jul 25 at 21:51
    
I've been considering AutoMapper as well. –  Aetherix Jul 28 at 9:19

4 Answers 4

There's nothing wrong with this kind of assignment logic per se, but you usually don't want it cluttering up the rest of your application logic so it usually belongs in a constructor method. If you're using the GetBreakDownsReport() method to construct the source for a datagrid, replace your anonymous object with a BreakdownsReportViewModel object and make a constructor that handles all property assignments. This will have the added benefit of making your GetBreakdownsReport() method signature more expressive and easier to read.

share|improve this answer
  • About the anonymous type: It depends on whether you need a name further on in your processing. If you're just going to keep everything in your flattened list, there's no need.
  • About copying all the elements: It is not clear whether the listed members are the only members of the child objects. If this is a specific flattening, where you are hand picking the elements you want, there's not much choice. If you are just taking everything, there might be a shortcut. It's possible you could do some reflection magic. This would keep your code consistent if the model were to change.
share|improve this answer

If the right-value is a field or property that have the same name you want to give, you can omit the left side part and the equal sign.

(b, bdl) => new
{
    //these two lines will produce the same result
    //Structure = b.Account.Structure,
    b.Account.Structure,
}

Return a collection type like List<object> instead of an object. If the entire list is not required, a IEnumerable<object> will be preferred, as only the required portion of the list will be created.

Also consider moving the properties selection part into your presentation layer, and have it to wire up the properties they need. So that, you don't need to change both place when you decide to add/remove a property.

share|improve this answer

One thing that I do sometimes in cases like this is let the caller decide how to project the results (and to thus implicitly declare the return type).

public IEnumerable<T> GetBreakdownsReport<T>(Func<Breakdown, BreakdownLine, T> resultSelector){
    return CurrentSecurity.Breakdowns.SelectMany(b=>b.BreakdownLines, resultSelector);
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.