MVC, DI (dependency injection) and creating Model instance from Controller

Question

My Dispatcher is "choosing" correct Controller; then creating Controller's instance (DependencyInjectionContainer is passed to Controller constructor); then calling some Controller's method...

class UserController extends Controller
{

  public function __construct(DependencyInjectionContainer $injection) {
    $this->container = $injection;
  }

  public function detailsAction() {
    ...
  }

}

DependencyInjectionContainer contains DB adapter object, Config object etc. Now let's see what detailsAction() method contains...

public function detailsAction() {

  $model = new UserModel();
  $model->getDetails(12345);

}

As you see I'm creating new instance of UserModel and calling getDetails methods. Model's getDetails() method should connect to db to get information about user. To connect to DB UserModel should be able to access DB adapter.

What is the right way to pass DependencyInjectionContainer to the UserModel? I think that this way is wrong...

public function detailsAction() {

  $model = new UserModel($this->container);
  $model->getDetails(12345);

}

Answer 1

Instead of injecting the entire DI Container into your classes, you should inject only the dependencies you need.

Your UserController requires a DB Adapter (let's call this interface IDBAdapter). In C# this might look like this:

public class UserController
{
    private readonly IDBAdapter db;

    public UserController(IDBAdapter db)
    {
        if (db == null)
        {
            throw new ArgumentNullException("db");
        }

        this.db = db;
    }

    public void DetailsAction()
    {
        var model = new UserModel(this.db);
        model.GetDetails(12345);
    }
}

In this case we are injectiing the dependency into the UserModel. In most cases, however, I would tend to consider it a DI smell if the UserController only takes a dependency to pass it on, so a better approach might be for the UserController to take a dependency on an Abstract Factory like this one:

public interface IUserModelFactory
{
    UserModel Create();
}

In this variation, the UserController might look like this:

public class UserController
{
    private readonly IUserModelFactory factory;

    public UserController(IUserModelFactory factory)
    {
        if (factory == null)
        {
            throw new ArgumentNullException("factory");
        }

        this.factory = factory;
    }

    public void DetailsAction()
    {
        var model = this.factory.Create();
        model.GetDetails(12345);
    }
}

and you could define a concrete UserModelFactory that takes a dependency on IDBAdapter:

public class UserModelFactory : IUserModelFactory
{
    private readonly IDBAdapter db;

    public UserModelFactory(IDBAdapter db)
    {
        if (db == null)
        {
            throw new ArgumentNullException("db");
        }

        this.db = db;
    }

    public UserModel Create()
    {
        return new UserModel(this.db);
    }
}

This gives you better separation of concerns.

If you need more than one dependency, you just inject them through the constructor. When you start to get too many, it's a sign that you are violating the Single Responsibility Principle, and it's time to refactor to Aggregate Services.