How do you deal with dependencies that require the object you inject into?

Question

I'm currently trying to refactor some rather complicated object graph to use dependency injection. I'm using C# and StructureMap, but the problem isn't really technology specific. I understand the basic principle, but drawing blanks on how would I resolve a dependency like this:

public class Food
{    
    public Food(IIngredient ingredient)
    {       
        this.foodProcessor = new FoodProcessor(this);
    }

    // ... Lots of things to customize food

    public void BuildFood()
    {
        this.foodProcessor.Process();
    }
}

public class FoodProcessor
{
    public FoodProcessor(Food f)
    {
        this.food = f;
    }

    public void Process(){
        // yummy
    }
}

In that case, a FoodProcessor on its own doesn't make sense. I can't process null or empty food. On the other hand, new in ctors are not good (right?), and should be injected as a dependency.

Is the new okay in this case, or is it possible to refactor this without major changes to my application?

Answer 1

Take for example a Potato, which is arguably a type of Food. Generally, potatoes don't whip out a food processor and turn themselves into cottage pie when you put them next to ground beef and carrots. Yet, arguably that is what the example above is implying. When your object model suggests such silly things, it's usually a sign that you need a different/better abstraction.

I like that you chose ingredient. That definitely makes a lot of sense. What I don't like is that you're modelling food as all of its constituent ingredients, the tools necessary to make it, and the final product to be consumed. Food is in charge of too much, including preparing itself, which is what is creating your circular dependency. Furthermore, it doesn't really make sense to have a food processor take in Food. It should probably take in the ingredients and turn them into food. Instead, let me propose you split your Food class up to separate it's current responsibilities.

public class Recipe
{
    private List<IIngredient> ingredients;
    private FoodProcessor foodProcessor;

    public Food prepare() {
        return this.foodProcessor.process(this.ingredients);
    }
}

Now, with something vaguely like the class above, we've separated out one of the responsibilities of the Food class in order to break that dependency cycle. We've broken Food up into two classes: Recipe and Food. Recipe represents the creational aspects of food, while Food is now only responsible for the 'consumption' aspects of food.

TLDR; When you run into dependency cycles like this one, look at your object model. Generally you're going to find you have some participants that are responsible for too much, which generally comes from your object model not being an accurate enough representation of what you're modelling.

Answer 2

It looks to me like you've violated the single responsibility principle. If a food is really meant to be a container for the ingredients, it has no business making itself (it also tightly couples your food processor class to your food class). Making the food is the job of the food processor.

If a food processor can't handle a null or empty food, make that contract explicit. Throw an exception if someone tries to use it that way. Another option is to disallow arbitrary creation of a food processor and make it go through some factory method which can enforce a complicated contract / relationship.

Answer 3

First things first, the other answers are absolutely correct that the kind of bi-directional dependency is usually a code smell, and if you can refactor it away you should do it.

That said, if you are unable to refactor it out (for instance if doing so would require large scale changes to the application) StructureMap and other DI containers often support Lazy Resolution.

Lazy<IFoodProcessor> foodProcessor;
public Food(IIngredient ingredient, Lazy<IFoodProcessor> foodProcessor) //or Func<IFoodProcessor>
{       
    this.foodProcessor = foodProcessor;
}
public void BuildFood()
{
    this.foodProcessor.Value.Process();
}

What this does is put off getting the actual foodProcessor instance until it is first used. And by making your IFood for the FoodProcessor Lazy as well, both will be constructable by your container.

Answer 4

Quick aside: OOP concepts are often explained in terms of real-world objects like cars and shapes, but this isn't a great way to think when actually writing object oriented code as things tend to get very hand-wavy. Instead think about what your program actually needs to do and what information it needs - this is what your objects should represent.

• The fact that Food is a noun rather than a verb implies to me that Food is a bunch of properties that encapsulates some information about food rather than existing to perform some task. This is reinforced by your comment "Lots of things to customize food". Presumably this is because your program captures this information in one part of the program (e.g. from a file or UI) and uses this information elsewhere.

• The name FoodProcessor along with the method Process implies that this object encapsulates some behaviour or task. I guess a real food processor would be chopping Food up into smaller pieces, but it doesn't really make sense to me why a computer program would do this so I'll just assume its working out the cost of ingredients or something.

If FoodProcessor is actually some sort of factory class used to "create" Food then Dogs answer is spot on.

Now onto your question:

1. As you are seeing, you want to avoid cyclic dependencies between objects. You definitely want to avoid cyclically dependent fields (as in your example), and all other things being equal I find it better to also avoid cyclic source code dependencies.

2. As long as the FoodProcessor is responsible for performing some task on Food its clear that the processor object needs a dependency on the object being processed. The object being processed however doesn't really need a dependency on the object processing it, the BuildFood method in your example is largely syntactic sugar and could be a replaced with an extension method.

3. When thinking about OOP in terms of real-world objects its tempting to put all of the things that you can "do" with an object as methods on that object (e.g you can prepare food, eat food, email pictures of food etc...). In practice this tends to be a bad idea as it means you need to create dependencies for all those processes in order to create a Food object, even if you aren't going to call all those processes. This is another reason why its best for a process to depend on the object it processes and not the other way around.

4. As an aside, when you have a class whose job it is to process some object I find its best to pass that object as an argument to its process method, rather than as a constructor dependency. This lets the same FoodProcessor instance process multiple items.

To summarise, I'd probably get rid of the BuildFood method and break the dependency on FoodProcessor.

On the other hand, new in ctors are not good (right?)

new in a constructor is not automatically bad, e.g. a class like DataTable will create new lists for the rows and columns. The reason why this is fine is because users of the DataTable class don't want to be able to replace the rows collection with their own one.

You should consider whether any users of your class will want to pass in their own replacement implementation of that dependency (don't forget unit tests). If you never want to inject an alternative implementation then new away (its really easy to refactor a constructor to accept a dependency if you find yourself needing it in the future). If 99% of the time some "default" dependency is used, maybe do something like this:

public class Foo
{
    private readonly IBar _bar;

    public Foo(IBar bar = null)
    {
        _bar = bar ?? new Bar();
    }
}