4
\$\begingroup\$

Writing my first ASP.NET Web API web service. Still learing the power of Linq as well. I feel like there is a more ideomatic way of doing this:

public class StylesController : ApiController
{
    private Entities db = new Entities();

    // GET api/Styles
    public IEnumerable<Style> GetStyles(int division = 0, int family = 0, int model = 0, int year = 0, string market = "ANY")
    {
        market = market.ToUpper(); // make sure market is upper case

        IEnumerable<Style> q = db.Styles;

        if (division > 0) { q = q.Where(s => s.DivisionId == division); }
        if (family > 0) { q = q.Where(s => s.FamilyId == family); }
        if (model > 0) { q = q.Where(s => s.ModelId == model); }
        if (year > 0) { q = q.Where(s => s.Year == year); }
        if (market != "ANY") { q = q.Where(s => s.MarketCode == market); }

        return q.Distinct().OrderBy(s => s.Id).ToList();
    }
}
\$\endgroup\$
3
  • \$\begingroup\$ From what I can see of this code, you are doing an AND operation. i.e. division = 1 AND year = 2012. Am I correct? \$\endgroup\$ Oct 10 '12 at 17:20
  • \$\begingroup\$ Correct, but each part of the AND is optional. \$\endgroup\$
    – bejumi
    Oct 10 '12 at 19:55
  • \$\begingroup\$ Fine to return an IEnumerable but that first declaration should be IQueryable<Style> q = \$\endgroup\$
    – Turnkey
    Oct 11 '12 at 23:29
6
\$\begingroup\$

Good layout and white space.

Although the logic works, I think it is very confusing, and not overly obvious. At first glance, it the if statements are very confusing. I would also rename the variables to have an Id or Code to the end of them to keep naming consistent.

I think this is a good place where you could incorporate the Specification Pattern.

public class StylesController : ApiController
{
    private Entities db = new Entities();

    public IEnumerable<Style> GetStyles(int divisionId = 0, int familyId = 0, int modelId = 0, int year = 0, string marketCode = "ANY")
    {
        var divisionIsAMatch = new DivisionIsAMatchSpecification(divisionId);
        var familyIsAMatch = new FamilyIsAMatchSpecification(familyId);
        var modelIsAMatch = new ModelIsAMatchSpecification(modelId);
        var yearIsAMatch = new YearIsAMatchSpecification(year);
        var marketIsAMatch = new MarketIsAMatchSpecification(marketCode);

        var q = db.Styles

        return q.Where(
                divisionIsAMatch
                .And(familyIsAMatch)
                .And(modelIsAMatch)
                .And(yearIsAMatch)
                .And(marketIsAMatch).IsSatisfiedBy
            ).Distinct()
           .OrderBy(s => s.Id);
    }
}

Where

internal class DivisionIsAMatchSpecification : Specification<Style>
{
    private readonly int _divisionId;

    public DivisionIsAMatchSpecification(int divisionId)
    {
        _divisionId = divisionId;
    }

    public bool IsSatisfiedBy(Style style)
    {
        if (_divisionId == 0) 
        {
            return true;
        }

        return style.DivisionId == _divisionId;
    }
}

Just rinse and repeat for each of the specifications you need. Of course, you will have to create the base classes for them too.

I haven't tested this, but I have used this pattern before, and it works really nicely.

\$\endgroup\$
1
  • \$\begingroup\$ I know it's an old answer, but I'm not happy with this approach. I'm not sure how that should be translated to database statements using LINQ. \$\endgroup\$ Mar 26 '18 at 7:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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