Should my Mapper class be injected into my controller or can I instantiate it directly?

Question

In my controllers, all dependencies are received through injection, following the Dependency inversion principle, except for one, the Mapper class, which is instantiated by the constructor:

public class HomeController : Controller
{
    private readonly ISomeAppService SomeAppService;
    private readonly Mapper Mapper;

    public HomeController(ISomeAppService someAppService)
    {
        SomeAppService = someAppService;
        Mapper = new Mapper();
    }

    public ActionResult Index()
    {
        var someList = SomeAppService.GetSomeList();
        var someListDTO = Mapper.Map(someList);
        return View(new HomeIndexViewModel(someListDTO));
    }

The SomeAppService is an Application Layer's Service in front of my Domain Layer. The Mapper receives domain objects and returns DTOs to be used by the view.

My reasoning is that since a ViewModel represents only the data that you want to display on your view/page, I can't foresee any situation where I would need to replace a Mapper with something else, or how this could be bad for testing. I also can't see this Mapper class being reused on any other Presentation Layer, since other views can be different from the Web presentation. To me it feels as a part of the Controller.

The question is, is this correct? Do I need to receive the Mapper through dependency injection? Do I need to define an interface for it? And if so, for what reasons? I want to adhere to SOLID principles, but I want to know if and how they apply here.

One way to think about this is to ask yourself what you do sans mapper class. Most people write a private function that does the job, or perhaps if the object is simple enough they just new it up and do the mapping inline. However, the reason THIS particular case looks fishy, is that your class is called "Mapper", doesn't take in any particular initialization (instructing it the objects being mapped) so I can only presume it has the knowledge to map ALL kinds of objects, perhaps some unrelated to this controller. That could be the reason this feels a tad smelly. — Trevor Ash, Sep 27 '15 at 23:09

Joseph Woodward · Accepted Answer · 2015-09-29T20:22:10.680

Personally I would recommend injecting your Mapper instance. This is exactly what IoC Containers are designed to manage.

There's no reason for it to be instantiated within your controller and in its current state violates both the open-close principle and inversion of control of the SOLID principles.

The benefits you'll gain when injecting it using your IoC container are:

Improved testability

By injecting your Mapper instance into your controller you'll be able to create mocks for it to write better tests. Whilst you can test your controller as it is now, you're unable to test your mapper instance within the controller for any conditions.

Improved extensibility

What happens in a few months time when you want to be able to pass a constructor argument into your mapper? You'll need to go through all of your controller actions and update your constructor. By passing the responsibility of creating your Mapper instances to your IoC container you're creating a single configuration point meaning any further modifications or changes to your mapper class can be managed and configured within one place.

There's one thing you can be sure about most software that's expected to live for longer than a few months - it will change. Whilst you may no see reasons for your mapper instance to change right now, it's good practice to design software in a way that enables you to make changes as easily as possible. The more bits and pieces you have to change, the most likely the chance of you breaking something or introducing a bug.

Thanks for the great answer. It never came to me that I might need to pass arguments to the mapper at some point, that was a good example. I'll definitely inject it into my controller. One thing wasn't very clear to me, what do you mean by "as it is now, you're unable to test your mapper instance within the controller for any conditions"? — Marcos Dimitrio, Sep 29 '15 at 01:25
Well spotted! Do you not have enough rep. to edit posts yet? — Joseph Woodward, Sep 29 '15 at 20:23
Fair enough :) To answer your question - As your mapper instance is instantiated inside of your controller and not injected, you're unable to mock it; so, for instance, you'd be unable to mock it to write a unit test testing how your controller handles `Mapper.Map` returning a null reference. — Joseph Woodward, Sep 29 '15 at 20:33

score 0 · Answer 2 · answered Sep 28 '15 at 05:07

Along with all the benefits, @JoeMighty gave above, I would add that by injecting the Mapper in your controllers, you could externalize Mapper.Map definitions, freeing controllers of that responsibility. Also, you could cache the mapping definitions by controlling the lifetime of the injected Mapper, so Mapper doesn't have to create the definitions on every request.

Should my Mapper class be injected into my controller or can I instantiate it directly?

2 Answers2