0

I have a few objects that should communicate between each other. I also want to have my code unit tested. I am questioning how I should handle their communication:

  • Should I make one instantiate the other one?
  • Should I pass one in the constructor of the other one?
  • Should I make them communicate via events/a observable pattern?

What criteria should I use to choose between those options?

To illustrate this, let me give an example for each one of those approaches:


Example of one object instantiating another one

Code Sample:

public class Restaurant
{
    private Kitchen _kitchen;

    public Restaurant()
    {
        this._kitchen = new Kitchen();
    }

    public void Prepare(Order order)
    {
        foreach(Meal meal in order.Meals)
            this._kitchen.Cook(meal);
    }
}

public class Kitchen
{
    public void Cook(Meal meal)
    {
         // cooking magic goes here
    }

}

public class App
{
    public void Main()
    {
        Restaurant restaurant = new Restaurant();

        restaurant.Prepare(new Order());
    }
}

Pros:

  • App does not know about Kitchen

Cons:

  • Unit testing Restaurant now requires testing for the behavior produced by Kitchen

Example using dependency injection via constructor

Code Sample:

public class Restaurant
{
    private ICooker _cooker;

    public Restaurant(ICooker cooker)
    {
        this._cooker = cooker;
    }

    public void Prepare(Order order)
    {
        foreach(Meal meal in order.Meals)
            this._cooker.Cook(meal);
    }
}

public class Kitchen: ICooker
{
    public void Cook(Meal meal)
    {
         // cooking magic goes here
    }

}

public interface ICooker
{
    void Cook(Meal meal);
}

public class App
{
    public void Main()
    {
        Kitchen kitchen = new Kitchen();
        Restaurant restaurant = new Restaurant(kitchen);

        restaurant.Prepare(new Order());
    }
}

Pros

  • Kitchen can be unit tested separately from Restaurant.
  • Inversion of control

Cons:

  • App now has to know how to instantiate Kitchen.
  • There is still some coupling between Restaurant and Kitchen.
  • Mocking is necessary when unit testing Restaurant

Example using dependency via events

public class Restaurant
{
    public event Action<Meal> Cooked;

    public void Prepare(Order order)
    {
        foreach(Meal meal in order.Meals)
            this.Cooked?.Invoke(meal);

    }
}

public class Kitchen
{
    public void Cook(Meal meal)
    {
         // cooking magic goes here
    }

}

public class App
{
    public void Main()
    {
        Kitchen kitchen = new Kitchen();
        Restaurant restaurant = new Restaurant();

        restaurant.MealCooked += kitchen.Cook;

        restaurant.Prepare(new Order());
    }
}

Pros:

  • Removes the necessity of mocking when unit testing Restaurant
  • Makes Restaurant and Kitchen really decoupled.

Cons:

  • App not only has to know how to instantiate Kitchen but also has to connect the related events and listeners.

Question

Is there a preferable approach? What criteria should I use to choose between those options?

  • In the events scenario, what are you calling the event listener you are using in your tests to verify that the Restaurant asks for each Meal to be cooked if it isn't a mock? – Bart van Ingen Schenau Jul 27 at 9:48
3

There is no one-size fits all answer here. Context is key.

First option: hardcoded private dependency

The main issue here is that you're hardcoding (and hiding) the Kitchen dependency of Restaurant. If you want to be able to unit test them separately, then you can't do that as you're now unable to test Restaurant without using Kitchen. In other words, any Kitchen failure is going to lead to a Restaurant unit test failure as well.

However, things change when you consider Kitchen to be a private implementation detail of Restaurant, in a way that the consumer (App) shouldn't even know whether a Restaurant uses a Kitchen or not. As a second requirement, Kitchen should be a sealed class to there's no polymorphism, since having a private hardcoded type makes it impossible to use derived subtypes of that dependency.

In that case, when you meet both requirements, a hardcoded dependency is acceptable. Note also that Restaurant unit tests can just use a real Kitchen, since it is considered to be part of the implementation.

Second option: Injected dependency

This is the most common way of doing things. It enables proper unit testing of each class, and it allows the top-level application to decide which ICooker should be injected into a Restaurant. That may be desirable if the consumer of your library has decided to create their own implementation of an ICooker, or a subtype of Kitchen.

I would suggest sticking with this option by default as it allows maximum configurability and testability, and only resort to the other options when you know for a fact that they are more applicable in your current situation.

Con App now has to know how to instantiate Kitchen

I'm confused why you're picking on the Kitchen instantiation and not the Restaurant instantiation that your top-level app would then logically also need to do.

It's either both or neither, depending on whether you consider a DI framework to resolve this issue or instead consider it as a simplified (but still applicable) way of "needing to know how to construct the objects".

And even in that case, it's not 100% true, as you can still delegate the DI registration logic to the library itself. In cases where there isn't much choice on how to stitch it all together, I generally let the library register its own dependencies, and the top-level application then calls that registration logic without needing to know exactly what it does:

// In App Startup.cs

public void ConfigureServices(IServiceCollection services)
{
    //register all needed dependencies from App, e.g.:
    services.AddRazorPages();

    //have the domain register itself:
    services.RegisterDomainDependencies();
}

// In Domain project

public static class ServiceCollectionExtensions
{
    public static IServiceCollection RegisterDomainDependencies(this IServiceCollection services)
    {
        //register all needed dependencies from domain, e.g.:
        services.AddTransient<IRestaurant, Restaurant>();
        services.AddTransient<ICooker, Kitchen>();

        return services;
    }
}

This means that the App doesn't really need to know how to bring it all together. It relies on the library to do it for the consumer. This is useful in cases where the consumer doesn't need to pick and choose dependencies. And even then, I may still develop some sort of builder pattern if the benefit outweighs the needed effort.

But the underlying tone here that knowing how to instantiate an objects is not correct in general. Dependency injection specifically exists to give that control to the consumer. As a wise uncle once said, with great power comes great responsibility.

Third option: app builds logical connection

This is in my opinion a weaker option than the previous one.

The only context in which I can imagine this being useful is if your library is really just a toolkit of unrelated tools, at which points the onus is on the consumer to use the tools as they see fit. That is not the case in your example, though.

Con App not only has to know how to instantiate Kitchen but also has to connect the related events and listeners.

This is a BIG drawback. For any sizable application, that's going to be a lot of stitching together. Additionally, this will have to be repeated for any other app that consumes your library.

If you make any changes to how these classes interact, you're going to be refactoring code for a long time, with a high risk of bugs.

Pro Makes Restaurant and Kitchen really decoupled.

It makes them too decoupled. You're edging really close to a microservice-like separation here. That can be valuable in cases where these can be considered independent domains with little to no overlap (domain-wise), but it's going to be overkill if you apply this level of separation to any and all classes in your domain logic.

This is, in my opinion, too decoupled, to a point of forcing your customer to repeatedly reinvent the wheel by implementing the glue between the components.

Keeping in mind that you developed your classes with an idea of how they should work together, that means you're either forcing your consumer to read your documentation or to figure out what you intended by themselves.

In essence, you're giving them considerably more work (compared to dependency injection which can be templated much more easily) for no real benefit.

Pro Removes the necessity of mocking when unit testing Restaurant

Never design your classes to suit your tests. This puts the cart before the horse. Tests should be written against existing objects, rather than structuring objects to fit your tests (or preferred method of testing).

The goal of a unit is to fulfill a purpose. The goal of a unit test is to confirm that the unit's purpose is being fulfilled. If you develop your unit based on your testing strategy, then the goal of a unit becomes to pass a test - which is not the right incentive.

You can start arguing that making a class testable by e.g. ensuring its dependencies are injected is also "preparing your unit to be tested", but there's a reasonable line to draw here. Writing test-friendly code in general is different from structuring a specific class in a way to suit a specific test you have in mind.

| improve this answer | |
  • 2
    I think the answer is spot on, except Never design your classes to suit your tests.. The whole point of TDD and BDD is designing tests before production code in order to improve its API design. That being said, sure: business logic (and hence, tests) should come before implementation details. – Albuquerque Jul 27 at 3:39
  • @Albuquerque: The advice is more related to building backdoors in your classes to facilitate certain testing strategies. Like I said, writing testable code globally speaking is perfectly fine - but tailoring classes to test strategies is not. Even in TDD/BDD, you still don't design your class to fit with your test. You design your interface, then your test, then your class. Testing comes before the implementation, but the implementation is done based on the interface, not the test itself. – Flater Jul 27 at 10:14
  • @Albuquerque: In short, "I'm going to implement my class [this way] because I'd prefer to (not) do [this] in my test" is the wrong order of operations on how your implementation and test should be designed. – Flater Jul 27 at 10:18
1

I'd go for dependency injection via constructor and/or parameter passing.

Let's debunk the cons:

App now has to know how to instantiate Kitchen.

There's no problem with that, as long as instantiation of concrete classes happen only in a factory class of factory methods (abstract factories, builders or any creational pattern that is suitable). Somewhere in the app some class has to be instantiated. Just keep this place centralized.

There is still some coupling between Restaurant and Kitchen.

Not really, Restaurant is coupled to the interface ICooker

Mocking is necessary when unit testing Restaurant

Yes, mocking is necessary for unit testing.

Also, I would change every reference to Order and Meal to corresponding references to IOrder and IMeal. I'd change exposed Meals to getMeals() not to break encapsulation.


public class Restaurant: IRestaurant
{
    private ICooker _cooker;

    public Restaurant(ICooker cooker)
    {
        this._cooker = cooker;
    }

    public void prepare(IOrder order)
    {
        foreach(IMeal meal in order.getMeals())
            this._cooker.Cook(meal);
    }
}

public class Kitchen: ICooker
{
    public void cook(IMeal meal)
    {
         // cooking magic goes here
    }

}

public interface ICooker
{
    void cook(IMeal meal);
}

public class App
{
    public void main()
    {
        ICooker kitchen = Factory.getICookerInstance();
        IRestaurant restaurant = Factory.getIRestaurantInstance(kitchen);
        restaurant.prepare(Factory.getOrderInstance());
    }
}

NOTE: All methods in the factory are static, return the type of an interface, but internally instantiate a concrete class. No concrete class knows about any other concrete class. Only the factory is coupled to all of them.

You could also use a DI framework but it could quite complex to configure (lots of annotations and XML files to edit).

| improve this answer | |
0

The Other Option

Dependency via a reasonably defaulted property.

class Kitchen
{

}

class Restaurant
{
    private Kitchen _kitchen;
    public Kitchen Kitchen
    {
        get
        {
            return _kitchen ?? (_kitchen = new Kitchen(/*reasonable defaults*/);
        }
        set
        {
             //an validation logic here
             _kitchen = value; 
        }
    }
}

Any client can use Restaurant with to much concern for its Kitchen, unless of course it is very much concerned with the Kitchen and can either directly manipulate it, or supply another Kitchen for use.

Best used with interfaces.

Con: Restaurant is aware of at least one implementation, or method to obtain a Kitchen.

| improve this answer | |

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.