4

I'm currently trying to refactor a C# MVC application in spite of having limited experience of the pattern. Reading around the subject, I seem to constantly blunder in to dimetric opposite opinions on best practice.

My biggest problem is that there's way too much stuff in the controllers. They work, but they're full of business logic which is hard to restructure and to test. Models are mostly just thin DTOs. So, where do I start putting this useful business logic in order to rework it and test it?

A lot of people say it should go in the model. But then you get some people saying it should go in the controller after all. And other people telling you that the principle of the model containing data and NOTHING ELSE is fundamental to the pattern.

Then you get people telling you it should go in a fourth type of class, a ViewModel. Now I've worked on MVVM in WPF and I'm familiar with this paradigm. But adding it to MVC just seems to replicate a lot of work that's done elsewhere, for no better reason than blindly following a pattern dictate.

Yet another option is to put it in some sort of helper class. This seems a common suggestion, which I won't link. But doing that seems a wasteful use of another class which has no point to exist outside of providing functions to a single controller. And that would seem a fundamental violation of OOP principles.

Is there a definitive "correct" answer to this? If so, why is there so much confusion? If not, how do you go about gauging the best solution in this morass of opinion?

Community
  • 1
  • 1
Bob Tway
  • 9,301
  • 17
  • 80
  • 162
  • You seem to have missed the most important one: *services*. This is what [SOLID](https://en.wikipedia.org/wiki/SOLID_(object-oriented_design)) is all about: abstracting away business logic into single-responsibility services that can be easily tested apart from (and injected into) each other and controllers. I highly recommend reading [Dependency Injection in .NET](https://www.manning.com/books/dependency-injection-in-dot-net) and [this article](http://misko.hevery.com/2008/09/30/to-new-or-not-to-new/) to understand how it all fits together in the context of a complex application. – NightOwl888 Jun 14 '16 at 14:33
  • Did you check out CQRS pattern? Basically your controllers can also be dumb. Passing commands to handlers that know how to execute those commands, and the same for queries. – Bishoy Jun 14 '16 at 15:14

3 Answers3

3

To me, the M in MVC stands for view model, it contains the data needed for the view to do its work, so that the view can remain as dumb as possible.

This view model is created in the controller and passed to the view, but as you said, the controller logic should be kept small - so no business logic here.

This business logic belongs in services - which could be remote services exposed with WCF, or a web API, or just a class library with methods in your web site.

So, summarized:

  1. the controller calls the services to get data in the form of a model. Business logic is inside these services, the model returned is just POCO.
  2. the controller creates a view model based on the received model, which contains the properties in the format which is best for the view.
  3. the controller passes the view model to the view, which uses the view model to build itself.
L-Four
  • 13,345
  • 9
  • 65
  • 109
  • Thanks for your suggestion. A class library for this is effectively the final approach I deal with in the question. Given that many of the functions in these classes won't be used elsewhere, isn't this wasteful in terms of code reuse? – Bob Tway Jun 14 '16 at 14:30
  • 1
    No, I would even recommend to program against interfaces, which allows you to easily mock services (testing purposes etc.). It's a valuable abstraction; and most likely some service operations will be reused in various controllers. – L-Four Jun 14 '16 at 14:33
2

I don't believe there is a definitive "correct" answer. I believe it's all in preference and how complex your data manipulation is.

At my company we use helper classes/a service for the purpose of organization. Our controllers do not contain any functionality except for taking in a model, manipulating it, and spitting it back to the view.

We have a vast amount of data manipulation, and to do that in the controller would be a horrible mess.

Most people would agree that no logic should be in the controller. The difference is where that logic should be. For my company, it would have been ridiculous to put it in the models themselves because of the sheer volume of manipulation that takes place. If there isn't a large amount of manipulation I would put it in the models.

Blue Eyed Behemoth
  • 3,692
  • 1
  • 17
  • 27
1

On the project I am currently working on, we have kept our Controllers reasonably free of Business Logic as we have separated them out into Units of Work. I believe this pattern was adopted to allow us to swap the Units of Work in and out depending on the client etc.

Where as the previous project I worked on was all in the controllers and broken down into smaller shared helpers where necessary.

I don't think there is strictly a correct way or an incorrect way. The majority of the time it seems to be down to personal preference of the developer.

Gaz Winter
  • 2,924
  • 2
  • 25
  • 47