Instantiating DbContext within ViewModel VS passing controller's instance

Question

I have an editing ViewModel that does some DB work.

Does it make any difference which of these approaches I take?

1. Pass in the controller's DBContext instance to the ViewModel (example 1 below)
2. Create a new DBContext instance within the ViewModel itself (example 2 below)

If I pass in the controller's DBContext instance, that means .Dispose() will be called on it at some point (assuming there is the .Dispose() method in the controller). By creating the DBContext instance within the ViewModel method itself, this never happens. Does that matter?

---

Example 1:

public class ModelOne
{
    public int ID { get; set; }
    public string PropA { get; set; }
    public string PropB { get; set; }
    public string PropC { get; set; }
    // etc...
}


public class EditModelOnePropAViewModel
{
    public EditModelOnePropAViewModel(ApplicationDbContext db, int id)
    {
        ID = id;
        PropA = db.ModelOneDbSet
            .Where(i => i.ID == id)
            .Select(i => i.PropA)
            .FirstOrDefault();
    }

    public void SaveChanges(ApplicationDbContext db)
    {
        var modelOne = db.ModelOneDbSet.FirstOrDefault(i => i.ID == ID);
        modelOne.PropA = PropA;
        db.SaveChanges();
    }

    public string PropA { get; set; }
    public int ID { get; set; }
}


public class ControllerOne : Controller
{
    private ApplicationDbContext DB = new ApplicationDbContext() { };

    [HttpGet]
    public ActionResult Edit(int id)
    {
        var viewModel = new EditModelOnePropAViewModel(DB, id);
        return View(viewModel);
    }
    [HttpPost]
    public ActionResult Edit(EditModelOnePropAViewModel postedModel)
    {
        if (ModelState.IsValid)
        {
            postedModel.SaveChanges(DB);
            return RedirectToAction("index");
        }
        return View(postedModel);
    }
}

---

Example 2:

public class ModelTwo
{
    public int ID { get; set; }
    public string PropA { get; set; }
    public string PropB { get; set; }
    public string PropC { get; set; }
    // etc...
}


public class EditModelTwoPropAViewModel
{
    public EditModelTwoPropAViewModel(int id)
    {
        using (var db = new ApplicationDbContext())
        {
            ID = id;
            PropA = db.ModelTwoDbSet
                .Where(i => i.ID == id)
                .Select(i => i.PropA)
                .FirstOrDefault();
        }
    }

    public void SaveChanges()
    {
        using (var db = new ApplicationDbContext())
        {
            var modelTwo = db.ModelTwoDbSet.FirstOrDefault(i => i.ID == ID);
            modelTwo.PropA = PropA;
            db.SaveChanges();
        }

    }
    public string PropA { get; set; }
    public int ID { get; set; }
}

public class ControllerTwo : Controller
{
    [HttpGet]
    public ActionResult Edit(int id)
    {
        var viewModel = new EditModelTwoPropAViewModel(id);
        return View(viewModel);
    }
    [HttpPost]
    public ActionResult Edit(EditModelTwoPropAViewModel postedModel)
    {
        if (ModelState.IsValid)
        {
            postedModel.SaveChanges();
            return RedirectToAction("index");
        }
        return View(postedModel);
    }
}

Answer 1

View models should be simple POCO's. Classes with properties which are needed for your view. Nothing else.

public class CustomerViewModel
{
  public int Id {set;get;}
  public string FirstName {set;get;}
  public string LastName {set;get;}
}

And in your controller,

public ActionResult Edit(int id)
{
  using(var db=new YourDbContext())
  {
     var c= db.Customers.FirstOrDefault(s=>s.Id==id);
     if(c!=null)
     {
       var vm= new CustomerViewModel { Id=id, 
                                       FirstName=c.FirstName,
                                       LastName=c.LastName
                                     };
      return View(vm);
     }
  }
  return View("NotFound");
}

Even better approach is creating an abstraction on your data access layer so that your controller code will not have any idea what data access technology you are using. This will help you to unit test your controller actions.

So basically you will create an abstraction like this

public interface ICustomerRepository
{
  CustomerDto GetCustomer(ind id);
}
public class EFCustomerRepository : ICustomerRepository
{
  public CustomerDto GetCustomer(int id)
  {
      using(var db=new YourDbContext())
      {
         var c= db.Customers.FirstOrDefault(s=>s.Id==id);
         if(c!=null)
         { 
          return new CustomerDto { Id=id, FirstName = c.FirstName };
         }
      }
      return null;
  }
}

Assuming you have a class called CustomerDto which is a data structure to represent the customer entity and is accessible to both the Web code and the data access code( You can keep in a Common Project and add reference to that in both the projects)

And in your controller, you will be using an implementation of the ICustomerRepository

public CustomerController : Controller
{
  ICustomerRepository repo;
  public CustomerController(ICustomerRepository repo)
  {
    this.repo =repo;
  }
  // You will use this.repo in your action methods now.
}

This will help you to use a fake implementation of ICustomerRepository in your unit tests. You can use mocking libraries like Moq/FakeItEasy to do that.

You can use a dependency injection framework like Unity,StructureMap or Ninject to inject the concrete implementation of the interface in your app.

Answer 2

A ViewModel is MVVM, not MVC. The Model in MVC is the same thing as the Model in MVVM. That pattern is called Model-View-Viewmodel

In MVC the controller is responsible for the page flow. Nothing else. A DbContext has nothing to do with page flow.

The Model is responsible for the business logic. A DbContext has a lot to do with the business logic. If you want to use the database this close to the presentation layer, the Model should create the DbContext.

Your controller example 2 is better than the one proposed by Shyju. Now you are able to test the page flow without being required to use a DbContext. And since the DbContext has nothing to do with page flow, it makes much more sense.

Some nitpicking: A POCO is not an object with public properties and no logic. It is a class that is not dependent on anything framework specific (e.g. a specific interface to implement).