Is it bad to use DI to inject constructor parameters at runtime?

Question

These are two classes, the first one I inject an instance of type IEngine while the second one I inject owner's name, tickets and engine.

Version 1:

public class Car
{
    public Car(IEngine engine)
    {
        // do something here
    }

    public void PrintOwnerName()
    {
    }
}

Version 2:

public class Car
{
    // owner's name, tickets history of this car, and the engine.
    public Car(string ownerName, List<string> tickets, IEngine engine)
    {
    }

    public void PrintOwnerName()
    {
    }
}

I was wondering if the version 2 is bad design. If so, why is it bad?

Answer 1

The decision which parameters a constructor should have is the same decision which parameters an arbitrary function should have - it should have exactly the parameters which are needed to create a specific, ideally easy to understand, abstraction. And if your abstraction of a car encapsulates exactly those three things, version 2 reflects that much better than version 1.

In your first version, you would have to provide owner name and tickets differently, probably by setting those properties afterwards. But if those properties must be always provided, it is IMHO a better design enforce this by constructor parameters.

One thing you have to care for is that the number of parameters passed through the constructor does not grow to a point where the code becomes hard to understand. For example, when you have a lot more parameters for your car like vendor, color, number of seats, number of doors, list of previous owners, needed driving licence for this type of car, and so on, then you should obviously not pass them all through the constructor (at least, not one by one). One approach to handle this can be to introduce one or more service facades. Another approach might be to add some properties (for things which are optional or might be changed later, as described in @Stephen's answer).

To make decision which is the best solution, however, one has to look at the actual parameters and their meaning, you cannot make a good decision just on "number and types" of the parameters. I assume your "car" example is just a placeholder for something different in your real program, for which the situation might be different, but to discuss it in terms of a car: the owner's name is typically a property of the owner, not of the car, and the list of tickets is AFAIK something associated with the owner, too. So you might consider to introduce an Owner class for this example, where the owner has a name and a list of tickets. So this leads to a constructor

 public Car(Owner owner, IEngine engine)

However, taking Stephen's advice into account, assuming the owner of a car can change over the lifetime of a car object, it might be indeed better to stick to your original version 1, provide a getter and a setter for the "current owner" and implement your PrintOwnerName() function with regards to the fact that the owner of the car might be "unknown" (which means uninitialized).

Answer 2

If a property meets the following criteria then I make it a constructor parameter:

• The class is dependent on it for operation
• It has no reason to change over the lifetime of the object
• Its value is known at object creation

If it meets these criteria, then it's a constructor parameter. If it only meets one or two of the criteria then it's almost certainly a settable property.

Constructor parameters are supposed to be used to inject the dependencies of the object. Use them for that and profit.

Answer 3

IMHO, it depends upon the parameters. I've used DI to inject cross cutting concerns, such as logging and session management. I think this is good use of DI as the magic of DI does not affect the class-specific logic.

Injecting instance specific parameters seems to be asking for trouble. Not the least of which is someone else trying to work out how your program works. Sometimes you write a bit of extra code to make the program simpler and easier to understand.

Answer 4

Version 1 is better than version 2. The reason for that is, a car should not know anything about an owner or any tickets. As cars are objects in our real world, think about it:

Can a car exist without having an owner or tickets? - Certainly yes.

Therefore, remove those from car and make car an independent class that is only dependent of what it is dependent in our world: an engine and possibly other technical properties.

Next, think about what determines the owner of a car. That would normally be a government-service that has a map to assign an owner to a car. Same goes for tickets. You can go even further and say that it does not even depend on that car itself but rather on the license that the government has given to the combination of the car and the owner.

But in your case a more simple modelation will be sufficient I guess, like having only one car-owner-service / car-ticket-service.

One of the many advantages of this modelation is for example, that you can give a college the car class so that he can use it anywhere he wants without having to know anything about tickets or having to deal with car owners. Furthermore, later you may come into a situation where you want a car to be able to have for example multiple owners, one for each country? Or maybe even more properties like a list of the ones allowed to drive it? In this case, you don't need to change the car class for that, but only the mentioned service. With your version 1 you are therefore violation the SRP.

In a nutshell: yes, version 2 is bad design. Go with the first version (and remove PrintOwnerName).