37

I'm working on an ASP.net MVC application and I have a question about using constructors for my controllers.

I'm using Entity Framework and linq to Entities for all of my data transactions. I need to access my Entity model for nearly all of my controller actions. When I first started writing the app I was creating an entity object at the beginning of each Action method, performing whatever work I needed to and then returning my result.

I realized that I was creating the same object over and over for each action method so I created a private member variable for the Entity object and started instantiating it in the constructor for each controller. Now each method only references that private member variable to do its work.

I'm still questioning myself on which way is right. I'm wondering A.) which method is most appropriate? B.) in the constructor method, how long are those objects living? C.) are there performance/integrity issues with the constructor method?

Luke Girvin
  • 13,221
  • 9
  • 64
  • 84
BZink
  • 7,687
  • 10
  • 37
  • 55

2 Answers2

33

You are asking the right questions.

A. It is definitely not appropriate to create this dependencies inside each action method. One of the main features of MVC is the ability to separate concerns. By loading up your controller with these dependencies, you are making the controller for thick. These should be injected into the controller. There are various options for dependency injection (DI). Generally these types of objects can be either injected into the constructor or into a property. My preference is constructor injection.

B. The lifetime of these objects will be determined by the garbage collector. GC is not deterministic. So if you have objects that have connections to resource constrained services (database connections) then you may need to be sure you close those connections your self (instead of relying on dispose). Many times the 'lifetime' concerns are separated out into an inversion of control (IOC) container. There are many out there. My preference is Ninject.

C. The instantiation costs are probably minimal. The database transactions cost are where you probably want to focus your attention. There is a concept called 'unit of work' you may want to look into. Essentially, a database can handle transactions larger than just one save/update operation. Increasing the transaction size can lead to better db performance.

Hope that gets you started.

Luke Girvin
  • 13,221
  • 9
  • 64
  • 84
rcravens
  • 8,320
  • 2
  • 33
  • 26
  • 1
    try NINJECT for Dependency Injection! – Omkar Dec 18 '10 at 22:54
  • 1
    "be sure you close those connections your self (instead of relying on dispose)" - I strongly disagree. IDisposable controllers get disposed by a ControllerFactory, not by the GC, and their disposal is deterministic. See [this question](http://stackoverflow.com/questions/1380019/asp-mvc-when-is-icontroller-dispose-called) for details. – Alex Sep 18 '15 at 13:28
31

RCravens has some excellent insights. I'd like to show how you can implement his suggestions.

It would be good to start by defining an interface for the data access class to implement:

public interface IPostRepository 
{
    IEnumerable<Post> GetMostRecentPosts(int blogId);
}

Then implement a data class. Entity Framework contexts are cheap to build, and you can get inconsistent behavior when you don't dispose of them, so I find it's usually better to pull the data you want into memory, and then dispose the context.

public class PostRepository : IPostRepository
{
    public IEnumerable<Post> GetMostRecentPosts(int blogId)
    {
        // A using statement makes sure the context is disposed quickly.
        using(var context = new BlogContext())
        {
            return context.Posts
                .Where(p => p.UserId == userId)
                .OrderByDescending(p => p.TimeStamp)
                .Take(10)
                // ToList ensures the values are in memory before disposing the context
                .ToList(); 
        }
    }
}

Now your controller can accept one of these repositories as a constructor argument:

public class BlogController : Controller
{
    private IPostRepository _postRepository;
    public BlogController(IPostRepository postRepository)
    {
        _postRepository = postRepository;
    }

    public ActionResult Index(int blogId)
    {
        var posts = _postRepository.GetMostRecentPosts(blogId);
        var model = new PostsModel { Posts = posts };
        if(!posts.Any()) {model.Message = "This blog doesn't have any posts yet";}
        return View("Posts", model);
    }

}

MVC allows you to use your own Controller Factory in lieu of the default, so you can specify that your IoC framework like Ninject decides how Controllers are created. You can set up your injection framework to know that when you ask for an IPostRepository it should create a PostRepository object.

One big advantage of this approach is that it makes your controllers unit-testable. For example, if you want to make sure that your model gets a Message when there are no posts, you can use a mocking framework like Moq to set up a scenario where your repository returns no posts:

var repositoryMock = new Mock<IPostRepository>();
repositoryMock.Setup(r => r.GetMostRecentPosts(1))
    .Returns(Enumerable.Empty<Post>());
var controller = new BlogController(repositoryMock.Object);
var result = (ViewResult)controller.Index(1);
Assert.IsFalse(string.IsNullOrEmpty(result.Model.Message));

This makes it easy to test the specific behavior you're expecting from your controller actions, without needing to set up your database or anything special like that. Unit tests like this are easy to write, deterministic (their pass/fail status is based on the code, not the database contents), and fast (you can often run a thousand of these in a second).

StriplingWarrior
  • 151,543
  • 27
  • 246
  • 315
  • That's great. Thanks for the help. I've had a similar model (DI) in the back of my mind for a while but I was still learning the ins-and-outs of MVC. I think it's time I go back and build for more sustainable controllers. Thanks a lot. – BZink Dec 20 '10 at 19:15
  • 3
    How would you send an argument to the constructor? I'm doing this the same way, except that in the constructor I only have `myRepository = new MyRepository()`. – Cody Jun 18 '13 at 21:29
  • @DoctorOreo: As I said in the post, MVC allows you to specify a controller factory. You can create a controller factory that knows to pass in the repository when it's creating your controller. Or you can use an existing Dependency-injection framework, set up the repository's bindings, and have MVC ask the DI framework to create the controller. – StriplingWarrior Jun 18 '13 at 22:00
  • @StriplingWarrior: Does your example work if and only there is an IoC in place or does the MVC framework automatically figures out instance of PostRepository, when we ask for IPostRepository. – user203687 Nov 18 '13 at 18:21
  • @user203687: You need to set up ASP.NET MVC with some kind of custom factory that knows how to create your controller--by default, it will only invoke a default constructor. You could write your own controller factory that defines how to construct each controller. But most DI frameworks have plugins that let you easily wire them up with ASP.NET, so in general you'll only see people using this pattern if they have an IoC container hooked up. – StriplingWarrior Nov 18 '13 at 20:55
  • actually I tried using this method in one of projects but it wouldn’t allow me to execute the controller. Apparently in MVC constructors in a controller must be paramertless. Is this a version issue? Why do I get that message? – Victor_Tlepshev Nov 08 '17 at 14:13
  • @Victor: Did you see my comment previous to yours? – StriplingWarrior Nov 08 '17 at 16:22
  • I didn't at the time of my comment but now I read it. That make sense – Victor_Tlepshev Nov 08 '17 at 20:00