MVC4 Best practices, where to include Logic

Question

I am learning MVC and have doubts on if I am coding my controllers and models in an inefficient way.

I have made the following examples to try and illustrate my case:

I tend to put most of my logic in my models. a simple example of this would be the following:

  public partial class TestModel
  {
    public List<TestObject> ReportData
    {
      get
      {
        TestRepository rep = new TestRepository ();
        return rep.GetData(IdObject);
      }
    }

    public int IdObject{ get; set; }
   }

this model generates the ReportData as soon as a valid IdObject is set. an advantage of this approach is that it can lead to smaller action methods.

  public class TestController
  {
    public ActionResult Test1()
    {
      return View(new TestModel());
    }

    [HttpGet]
    public ActionResult Test2()
    {
      return View(new TestModel());
    }

    [HttpPost]
    public ActionResult Test3(TestModel model)
    {
      return View(model);
    }
}

as opposed to :

  public class TestController
  {
    public ActionResult Test1()
    {
      TestModel model = new TestModel();
      TestRepository rep = new TestRepository ();
      model.ReportData = rep.GetData(IdObject);
      return View(model);
    }

    [HttpGet]
    public ActionResult Test2()
    {
      TestModel model = new TestModel();
      TestRepository rep = new TestRepository ();
      model.ReportData = rep.GetData(IdObject);
      return View(model);
    }

    [HttpPost]
    public ActionResult Test3(TestModel model)
    {
      TestRepository rep = new TestRepository ();
      model.ReportData = rep.GetData(IdObject);
      return View(model);
    }
}

so in the end I cut down on code reuse by sticking as much logic as possible in my model. An other upside in my eyes is that i can make most properties read only and forget about something or someone overwriting them (opened colsed principle).

the problem I am having with this approach is that sometimes properties can be costly to calculate (they may come from databases or might be processor intensive calculations) and there are times where the calculation is done multiple times.

for example if I have a view that includes the following code:

@if (Model.ReportData.Count() > 0)
                {
                    foreach (var item in Model.ReportData)
                    {
                        item
                    }
}

how can I make sure that the data is not calculated over and over again? and is there a best practice of some advice o how to better code my models and controllers?

score 2 · Answer 1 · answered Dec 05 '13 at 21:07

In general, logic should be in the controller, not the model. You can have controllers call shared private methods to avoid repeating yourself. Example:

Model:

  public partial class TestModel
  {
    public List<TestObject> ReportData { get; set; }
    public int IdObject{ get; set; }
  }

Controller:

public class TestController
  {
    public ActionResult Test1()
    {
      return View(CreateTestModel());
    }

    [HttpGet]
    public ActionResult Test2()
    {
      return View(CreateTestModel());
    }

    [HttpPost]
    public ActionResult Test3(TestModel model)
    {
      return View(CreateTestModel());
    }

    private TestModel CreateTestModel() {
      TestModel model = new TestModel();
      TestRepository rep = new TestRepository ();
      model.ReportData = rep.GetData(IdObject);
      return model
    }
}

This also solves your problem of not having any data calculated when your view accesses properties, because your controller populates the properties, so accessing them from the model does not result in re-calculation.

Good answer, +1 - might also be worth detailing *why* logic belongs in controllers instead of view models. — Ant P, Dec 05 '13 at 21:09
I would really benefit from an explanation on why the logic should be in the controller. I have searched around and there seems to be a lot of discussion in favor of both approaches. for example http://stackoverflow.com/questions/235233/asp-net-mvc-should-business-logic-exist-in-controllers makes a case for fat models and skinny controllers — mmilan, Dec 05 '13 at 21:25
I use the models as a description of my data, and the controllers are responsible for updating the models. However in this approach you will end up with huge controllers, so I use a repository pattern (or data-layer). — Marthijn, Dec 06 '13 at 07:28

score 1 · Accepted Answer · answered Dec 06 '13 at 20:02

By far, the best answer I have seen to this question is in Steve Smith's ASP.NET MVC Solution Best Practices Presentation at ASPConf2012. Although it may seem like a "long video", it is well worth it.

In it, Smith takes an existing solution and refactors the code to follow the "Onion Architecture" and shows you what code artifacts belong where in the MVC Architecture. Seeing the "before" and "after" codebase helps a lot in understanding the "why".

enter image description here

Here's the source code for the sample application that Smith goes over in his Presentation.

Thank you for the contribution. I will check the video out during lunch! — mmilan, Dec 09 '13 at 15:54

MVC4 Best practices, where to include Logic

2 Answers2