2

I am using a Unit of Work pattern in my mvc 3 app with Ninject. I have run into a problem where i'm having difficulty solving it without using new or service location of some kind.

I am using an abstract base model called MyModel which has 2 concrete subclasses MyModel1 and MyModel2. These need to get sent to the view based on a value set in the users record.

public class MyController : Controller
{
    private IUnitOfWork _unitOfWork;
    public MyController(IUnitOfWork unitOfWork) { _unitOfWork = unitOfWork; }

    ...

    public ActionResult MyMethod() {
        var user = _unitOfWork.Get(user)

        if (user.myprop == somevalue)
        {
            return View(new MyModel1());
        }

        return View(new MyModel2());
    }

This works fine (although I don't like it, it's simple and works. The problem is that using new is an anti-pattern when using Dependancy Injection. Additionally, I now want the models to initialize themselves (from the database) so I need to inject the IUnitOfWork into the constructor of the models. I could, of course do this:

if (user.myprop == somevalue)
{
    return View(DependancyResolver.Current.GetService(typeof(MyModel1)));
}

But Service Location is also an anti-pattern.

Any suggestions on how to solve this?

Erik Funkenbusch
  • 92,674
  • 28
  • 195
  • 291
  • http://stackoverflow.com/questions/1943576/is-there-a-pattern-for-initializing-objects-created-via-a-di-container/1945023#1945023 – Mark Seemann Aug 31 '11 at 19:15

2 Answers2

3

Using new is not an anti pattern for DI if used correctly. There is absolutely no problem to use new to create data containers such as view models.

But it is an anti pattern for MVC applications to have logic or data retrieving code in your view models so that they need dependencies. All this stuff belongs outside into the controller or some services. The data is assigned preformatted to the view model from outside.

Remo Gloor
  • 32,665
  • 4
  • 68
  • 98
2

Additionally, I now want the models to initialize themselves (from the database) so I need to inject the IUnitOfWork into the constructor of the models

No. You should not pass any models to your views. You pass VIEW MODELS only. View models are dumb. They only contain preformatted data for the view to display. If you used AutoMapper for example you could have externalized this into the mapping layer and your controller could become:

public ActionResult MyMethod() {
    var user = _unitOfWork.Get(user)
    var userViewModel = Mapper.Map<User, UserViewModel>(user);
    return View(userViewModel);
}
Darin Dimitrov
  • 1,023,142
  • 271
  • 3,287
  • 2,928
  • I wasn't clear, I was referring to my view models. I want to initialize things like SelectLists for the view from the database. Perhaps you're right though, I shouldn't have to do a database lookup every time a view model is created. – Erik Funkenbusch Aug 31 '11 at 18:35
  • @Mystere Man, it's not the responsibility of view models to initialize anything. Ideally this should be done in the mapping layer when converting between your domain models to view models. – Darin Dimitrov Aug 31 '11 at 18:35
  • So i'm not sure how AutoMapper can fill in the SelectLists when they're not part of the User model. I guess that's just my lack of familiarity with AutoMapper – Erik Funkenbusch Aug 31 '11 at 18:40
  • @Mystere Man, `AutoMapper` converts between one type to another. You could define rules for this conversion. So for example if in your User model you have a list of some key/value pairs, it could be transformed into a SelectList. Checkout the documentation: https://github.com/AutoMapper/AutoMapper/wiki – Darin Dimitrov Aug 31 '11 at 18:43
  • Yeah, reading the somewhat sparse documentation doesn't really help. I need to 1) Create eiter a MyModel1 or MyModel2 based on the value of user.prop, then I need to fill a SelectList using the IUnitOfWork to retrieve data from a repository in my dal for the correctly chosen view model so i can hand it to the view. I don't see how AutoMapper helps me in the critical part of creating the right ViewModel instance based on the user data without using new or service location. – Erik Funkenbusch Aug 31 '11 at 19:13
  • @Mystere Man, do `MyModel1` and `MyModel2` share a common ancestor? – Darin Dimitrov Aug 31 '11 at 19:18
  • @DarinDimitrov let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/3038/discussion-between-mystere-man-and-darin-dimitrov) – Erik Funkenbusch Aug 31 '11 at 19:21