Question

I had asked this question previously on SO. This is related to it. We have code base similar to this:

IRecipie FindRecipiesYouCanMake(IEnumerable<Ingredientes> stuff, Cook cook)
{
 if(stuff.Any(s=>s.Eggs && s.Flour) && cook.DinerCook)
 {
  if(s=>s.Sugar)
   return new Pancake("Yum");
  if(s=>s.Salt)
   return new Omlette("Yay");
 }
 /*.....
 ......
 .....
 loads of ifs and buts and else*/
}

I want to get rid of this mess and take a more data structure and OO route. Even the code sample i have provided is not as horrendous as it is. I looked at the specification pattern and found it applicable. Any ideas how to improve the code.

EDIT: Now that I realize it, I might even like to implement a method of this signature:

List<IRecipe> WhatAllCanBeCooked(IEnumerable<Ingredients> stuff, Cook cook);
Was it helpful?

Solution

I would delegate this logic to the individual IRecipie classes:

if (Pancake.CanBeMadeBy(stuff, cook)) {
    return new Pancake("Yum");
}
....


public class Pancake: IRecipe {
    ...
    public static bool CanBeMadeBy(IEnumerable<Ingredientes> stuff, Cook cook) {
        return stuff.Any(s=>s.Eggs && s.Flour && s.Sugar) && cook.DinerCook;
    }

}

Edit in response to comment

To find all the recipes that can be cooked, just do something like this:

List<IRecipe> results = new List<IRecipe>();

if (Pancake.CanBeMadeBy(stuff, cook)) {
    results.Add(new Pancake("Yum");
}
....

Edit 2 Alternatively, if you store a list of all possible recipes somewhere, you can turn CanBeMadeBy into an instance method instead of a static, and do this:

List<IRecipe> allRecipes = // all possible recipes
...
return allRecipes.Where(r => r.CanBeMadeBy(stuff, cook));

OTHER TIPS

Some ideas:

  • use decision tables

  • use the strategy pattern. This helps you to encapsulate a group of actions or parameters belonging together in different concrete classes. Once you have decided which strategy to use, you don't need any 'ifs' any more to dispatch between the strategies.

EDIT: some additional ideas:

  • start "small": most often, just simple refactoring to smaller, well-named, reusable functions will help you to reduce the if-else-if-else-soup. Sometimes, a simple, well named boolean variable does the trick. Both are examples for refactorings you will find in Fowler's book "Refactoring".

  • think "big": if you have really lot of complex business rules, constructing a "domain specific language" is an option that can sometimes be the right way of getting the complexity down. You will find lots of material on this topic just by googling for it. Citing David Wheeler All problems in computer science can be solved by another level of indirection.

ORIGIINAL POST -- Martin Fowler has solved this problem for you... its called the Specification pattern.
http://en.wikipedia.org/wiki/Specification_pattern

UPDATED POST --

Consider using the Composite Specification Pattern when:

  • You need to select a subset of objects based on some criteria,
  • You need to check that only suitable objects are used for a certain role, or
  • You need to describe what an object might do, without explaining the details of how the object does it

The true power of the pattern is in the ability to combine different specifications into composites with AND, OR and NOT relationships. Combining the different specifications together can be done at design time or runtime.

Eric Evan book on Domain Driven Design has an excellent example of this pattern (the Shipping Manifest)

This is the Wiki link:

http://en.wikipedia.org/wiki/Specification_pattern

At the bottom of the wiki link is this PDF link:

http://martinfowler.com/apsupp/spec.pdf

I think what that block of code is essentially trying to accomplish is linking recipes to ingredients within that recipe. One approach would be to include a list of ingredients on the recipe class itself, and then compare that against the list of ingredients passed in, like so:

public interface IRecipe {
   IEnumerable<Ingredient> Ingredients { get; }
}

public class Omlette : IRecipe {
   public IEnumerable<Ingredient> Ingredients { 
      get {
         return new Ingredient[]{new Ingredient("Salt"), new Ingredient("Egg")};
      }
   }
}

// etc. for your other classes.

IRecipie FindRecipiesYouCanMake(IEnumerable<Ingredientes> stuff, Cook cook)
{
    var query = Recipes.Where(r => !r.Ingredients.Except(stuff).Any());
    return query.First();
}

This assumes that you have a collection somewhere of all your recipes. It should be simple enough to set up a static list of those or pull from a database however.

The Linq query in question looks for any recipes where all ingredients passed in stuff are present in the list of ingredients (or, as it's stated, there are no ingredients in Ingredients that are not in stuff). This also might decrease the need to have subclasses for Recipes, which seems a bit odd (although for all I know there's additional reasons you'll require this)

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top