Search with filters code

Question

I have this method for searching a DB based on filters I was wondering if anyone thought as I do that this code is excessive, and secondly if you have any suggestions to shorten the code.

private const string NO_FILTER = "No filter";

[DataObjectMethod(DataObjectMethodType.Select)]
public static List<Part> GetPartsSearch(string partNumber, string modelNumber, string slotNumber, string yardNumber, int commodityId, string description)
{
    using (var context = new EntitiesModel())
    {
        IQueryable<Part> parts;

        if (slotNumber != NO_FILTER)
        {
            if (modelNumber != "0")
            {
                if (yardNumber != NO_FILTER)
                {
                    if (commodityId > 0)
                    {
                        // Have slot, model, yard and commodity
                        parts = from part in context.Parts
                                join partModel in context.PartsByModels on part.PartNumber equals
                                    partModel.PartNumber
                                join partSlot in context.PartsBySlots on part.PartNumber equals
                                    partSlot.PartNumber
                                join partYard in context.PartsByYards on part.PartNumber equals
                                    partYard.PartNumber
                                join commodity in context.Commodities on part.CommodityId equals
                                    commodity.CommodityID
                                where partSlot.SlotNumber == slotNumber
                                      && partModel.ModelNumber == modelNumber
                                      && partYard.YardNumber == yardNumber
                                      && commodity.CommodityID == commodityId
                                select part;
                    }
                    else
                    {
                        //Have slot, model and yard
                        parts = from part in context.Parts
                                join partModel in context.PartsByModels on part.PartNumber equals
                                    partModel.PartNumber
                                join partSlot in context.PartsBySlots on part.PartNumber equals
                                    partSlot.PartNumber
                                join partYard in context.PartsByYards on part.PartNumber equals
                                    partYard.PartNumber
                                where partSlot.SlotNumber == slotNumber
                                      && partModel.ModelNumber == modelNumber
                                      && partYard.YardNumber == yardNumber
                                select part;
                    }
                }
                else
                {
                    //have slot, model and commodity
                    if (commodityId > 0)
                    {
                        parts = from part in context.Parts
                                join partModel in context.PartsByModels on part.PartNumber equals
                                    partModel.PartNumber
                                join partSlot in context.PartsBySlots on part.PartNumber equals
                                    partSlot.PartNumber
                                join commodity in context.Commodities on part.CommodityId equals
                                    commodity.CommodityID
                                where partSlot.SlotNumber == slotNumber
                                      && partModel.ModelNumber == modelNumber
                                      && commodity.CommodityID == commodityId
                                select part;
                    }
                    else
                    {
                        //Have slot and model
                        parts = from part in context.Parts
                                join partModel in context.PartsByModels on part.PartNumber equals
                                    partModel.PartNumber
                                join partSlot in context.PartsBySlots on part.PartNumber equals
                                    partSlot.PartNumber
                                where partSlot.SlotNumber == slotNumber
                                      && partModel.ModelNumber == modelNumber
                                select part;
                    }
                }

            }
            else if (yardNumber != NO_FILTER)
            {
                if (commodityId > 0)
                {
                    //Have slot yard and commodity    
                    parts = from part in context.Parts
                            join partSlot in context.PartsBySlots on part.PartNumber equals
                                partSlot.PartNumber
                            join partYard in context.PartsByYards on part.PartNumber equals
                                partYard.PartNumber
                            join commodity in context.Commodities on part.CommodityId equals
                                   commodity.CommodityID
                            where partSlot.SlotNumber == slotNumber
                                  && partYard.YardNumber == yardNumber
                                  && commodity.CommodityID == commodityId
                            select part;
                }
                else
                {
                    //Have slot and yard
                    parts = from part in context.Parts
                            join partSlot in context.PartsBySlots on part.PartNumber equals
                                partSlot.PartNumber
                            join partYard in context.PartsByYards on part.PartNumber equals
                                partYard.PartNumber
                            where partSlot.SlotNumber == slotNumber
                                  && partYard.YardNumber == yardNumber
                            select part;
                }
            }
            else
            {
                if (commodityId > 0)
                {
                    parts = from part in context.Parts
                            join partSlot in context.PartsBySlots on part.PartNumber equals
                                partSlot.PartNumber
                            join commodity in context.Commodities on part.CommodityId equals
                                   commodity.CommodityID
                            where partSlot.SlotNumber == slotNumber
                                  && commodity.CommodityID == commodityId
                            select part;
                }
                else
                {
                    parts = from part in context.Parts
                            join partSlot in context.PartsBySlots on part.PartNumber equals
                                partSlot.PartNumber
                            where partSlot.SlotNumber == slotNumber
                            select part;
                }
            }
        }
        else if (modelNumber != "0")
        {
            if (yardNumber != NO_FILTER)
            {
                // Have model, yard and commodity
                if (commodityId > 0)
                {
                    parts = from part in context.Parts
                            join partModel in context.PartsByModels on part.PartNumber equals
                                partModel.PartNumber
                            join partYard in context.PartsByYards on part.PartNumber equals
                                partYard.PartNumber
                            join commodity in context.Commodities on part.CommodityId equals
                                  commodity.CommodityID
                            where partModel.ModelNumber == modelNumber
                                  && partYard.YardNumber == yardNumber
                                   && commodity.CommodityID == commodityId
                            select part;
                }
                else
                {
                    // Have model and yard
                    parts = from part in context.Parts
                            join partModel in context.PartsByModels on part.PartNumber equals
                                partModel.PartNumber
                            join partYard in context.PartsByYards on part.PartNumber equals
                                partYard.PartNumber
                            where partModel.ModelNumber == modelNumber
                                  && partYard.YardNumber == yardNumber
                            select part;
                }
            }
            else
            {
                if (commodityId > 0)
                {
                    // Have model and commodity
                    parts = from part in context.Parts
                            join partModel in context.PartsByModels on part.PartNumber equals
                                partModel.PartNumber
                            join commodity in context.Commodities on part.CommodityId equals
                                 commodity.CommodityID
                            where partModel.ModelNumber == modelNumber
                             && commodity.CommodityID == commodityId
                            select part;
                }
                else
                {
                    // Have model
                    parts = from part in context.Parts
                            join partModel in context.PartsByModels on part.PartNumber equals
                                partModel.PartNumber
                            where partModel.ModelNumber == modelNumber
                            select part;
                }
            }
        }
        else if (yardNumber != NO_FILTER)
        {
            if (commodityId > 0)
            {
                //have yard and commodity
                parts = from part in context.Parts
                        join partYard in context.PartsByYards on part.PartNumber equals
                            partYard.PartNumber
                        join commodity in context.Commodities on part.CommodityId equals
                                 commodity.CommodityID
                        where partYard.YardNumber == yardNumber
                          && commodity.CommodityID == commodityId
                        select part;
            }
            else
            {
                // Have yard
                parts = from part in context.Parts
                        join partYard in context.PartsByYards on part.PartNumber equals
                            partYard.PartNumber
                        where partYard.YardNumber == yardNumber
                        select part;
            }
        }
        else if (commodityId > 0)
        {
            // Have commodity
            parts = from part in context.Parts

                    join commodity in context.Commodities on part.CommodityId equals
                             commodity.CommodityID
                    where commodity.CommodityID == commodityId
                    select part;
        }
        else
        {
            parts = from part in context.Parts
                    select part;
        }

        if (partNumber != "0")
        {
            parts = parts.Where(p => p.PartNumber.ToLower().Contains(partNumber.ToLower()));
        }

        if(!string.IsNullOrEmpty(description))
        {
            parts = parts.Where(p => p.Description.ToLower().Contains(description.ToLower()));
        }

        parts = parts.OrderBy(p => p.PartNumber);

        return parts.ToList();
    }
}

Answer 1

It's definitely excessive. :)

I'd start by looking into whether there should be one method for getting the data without a commodity filter, and one method for the ones with commodity.

I'd also put each query into its own method anyway. That will give you quite a bit more readable main function(s).

But to be frank, I wonder why you don't have navigation properties on your entities, and just eagerload them by .Include (if EF) or use a DataLoadOptions set (if L2S). That way you could ditch all the joins from all the queries.

Also, if you change to the extension syntax, you can add where statements to the queryable.

For instance (EntityFramework eagerloading):

var queryable = context.Parts;
if (shouldIncludeModel)
    queryable = queryable.Include("Model");
if (model > 0)
    queryable = queryable.Where(p => p.Model.Id == model);
if (shouldIncludeYard)
   queryable = queryable.Include("Yard");
if (yard > 0)
    queryable = queryable.Where(p => p.Yard.Id == yard);

return queryable.ToList();

etc.

This should of course also be further refactored in many a better way, at least separated in methods responsible for each of the related entities. Anyway, I think you'll get quite a bit shorter methods by doing this.

Building on palacsints suggestion, you could also check out how to build Linq expressions manually, but that is quite advanced and cumbersome. There's some info here: http://msdn.microsoft.com/en-us/library/bb397951.aspx

Answer 2

I'd try creating a query builder class which stores the join statements and where conditions and later build the whole query from the stored statements. A sample pseudocode:

public class QueryBuilder {
    List<String> joins;
    List<String> whereConditions;

    ...

    public void addJoin(final String join) {
        joins.add(join);
    }

    public void addWhereCondition(final String condition) {
        whereConditions.add(condition);
    }

    public String buildQuery() {
        final StringBuilder result = new StringBuilder();
        result.append("...");
        for (final String join: joins) {
            result.append(join);
        }
        result.append("WHERE ");
        for (final String condition: whereConditions) {
            result.append(condition);
            // TODO: handle &&s here
        }

        return result.toString();
    }
}

I suppose something like this is possible in C# too. Then, the ifs could be rewritten like this:

if (commodityId > 0) {
    queryBuilder.addJoin("join commodity in context.Commodities on part.CommodityId 
        equals commodity.CommodityID");
    queryBuilder.addWhere("commodity.CommodityID == commodityId");
}

if (yardNumber != NO_FILTER) {
    queryBuilder.addJoin("join partYard in context.PartsByYards on part.PartNumber 
        equals partYard.PartNumber");
    queryBuilder.addWhere("partYard.YardNumber == yardNumber");
}

It would reduce cyclomatic complexity and make the code easier to read.