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

• JoeGeeky: Although I've not tried this with these specific constructs, you may want to look into Compilable Linq Expressions. These would be defined, build, and compiled in your constructor and reused throughout. This is a pattern I've used with similarly complex linq queries and acheived big performanace advantages. Also, as a micro-optimization, ToUpper() is faster then ToLower(). And lets not forget that offloading this to the database in a SPROC or View would be much-much faster and a better use of resources overall.

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

• JoeGeeky: I agree with the seperation, as it stands the JIT will not optimize this method because its just too large.

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.