HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Filtering units by a dynamic Lambda expression

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
expressionunitsdynamiclambdafiltering

Problem

I have a simple if statement which, based on user input, determines which expression is going to be utilized. It looks like it can be done better.

Func exp;
if (onlyChildren)
    exp = u => u.PhaseId == id;
else
    exp = u => u.PhaseId != id;

var unitGroups = _db.Units.Where(exp).GroupBy(m => m.UnitTemplate, (m, group) => new UnitTemplateZoneVm()
{
    Id = m.Id,
    Units = group.ToList()
}).ToList();

Solution

Don't omit braces. Ever. It's such a bad idea (I learned this the hard way).

As far as your lambda, your method is not necessarily a bad thing, though there are always other ways to write it.

var unitGroups = _db.Units
                    .Where(u => (onlyChildren && u.PhaseId == id) || (!onlyChildren && u.PhaseId != id))
                    .GroupBy(m => .UnitTemplate, (m, group) => new UnitTemplateZoneVm()
                    {
                        Id = m.Id,
                        Units = group.ToList()
                    })
                    .ToList();


Whitespace is free, use it. It makes it clearer as to what steps belong together.

I would personally prefer this method, as you can extract this out into LINQ-SQL style, instead of using the methods. Personally, I prefer the LINQ-SQL style rather than the methods, but the choice is up to you.

Code Snippets

var unitGroups = _db.Units
                    .Where(u => (onlyChildren && u.PhaseId == id) || (!onlyChildren && u.PhaseId != id))
                    .GroupBy(m => .UnitTemplate, (m, group) => new UnitTemplateZoneVm()
                    {
                        Id = m.Id,
                        Units = group.ToList()
                    })
                    .ToList();

Context

StackExchange Code Review Q#105774, answer score: 7

Revisions (0)

No revisions yet.