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

ASP.NET Web API HTTP GET with optional parameters

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

Problem

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 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 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();
    }
}

Solution

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 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
{
    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.

Code Snippets

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);
    }
}
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;
    }
}

Context

StackExchange Code Review Q#16410, answer score: 6

Revisions (0)

No revisions yet.