Keeping a controller thin (too many action methods)

Question

I'm working on my first real ASP.NET MVC project and I've noticed that the controller I've been working in is getting rather large. This seemingly goes against the best practice of keeping your controllers thin.

I've done a good job keeping the business logic out of the controllers. I use a separate layer for that. Each action primarily calls a method in the business layer and coordinates the end result based on whether or not the modelstate is valid.

That said, the controller has a large number of action methods. Intuitively, I would like to break the controller down into sub-controllers but I don't see an easy way to do that. I could simply break the controller down into separate controllers but the I loose the hierarchy and it feels a bit dirty.

Is it necessary to refactor a controller with a large number of thin actions? If so, what is the best way to do this?

Luscious

you can still break them into sub controllers in sub folders as well as the Views – Rolling

If its a web API then 5 actions. GET (all), GET (single) POST, PUT, and DELETE. For a MVC app, its a trickier question. – Stair

Answer 1

First, when you hear that it's good to keep controller code to a minimum, this mainly refers to keeping each action method as thin as possible (put logic instead into business classes, not into Views and ViewModels.) It seems you're doing this, which is great.

As for having "too many" action methods, this is a judgment call. It could actually be a sign of good organization, that you're having each action focus on one thing. Also, maybe you're using actions specifically for use with RenderAction? And, it could just be the nature of your solution that there are many things to do relating to your Controller's theme.

So, my guess is that you're probably fine. However, to make sure, on note paper break out the controller into 2 or 3 controllers, and sketch out how your stories would work moving from action to action. And if you find that your workflow works with more controllers, you should break it out. Especially if you're going to be adding to this functionality later. The sooner your break it out the better.

Stook

Answer 2

Good question.

I believe a "thin" controller may still need to be "wide" or "tall" depending on how you want to stretch the analogy. If there is no clean way to break up a controller that needs to do a lot of things, I don't think that's a problem as long as each Action is focused exclusively on preparing Views/ViewModels and is of limited code size.

Weisshorn

Answer 3

Another structural option you have is introducing partial classes for logical groupings of actions. And using something like vscommands to group the files together.

I doubt anybody can come up with a magic number of actions that tells you when it's a good idea to break stuff up and introduce new controllers, it really depends on your domain.

Heliograph

In my opinion using partial classes is just trying to cover up a code smell. If you can identify where to break the class into partials (via SRP), why not go all the way and just break up the class? – Talishatalisman

Answer 4

Another solution can be to separate the controllers based on the business concepts of the actions that are included in them, but the route of each action is in accordance with the REST rules.
However, I think it is better to use this solution only in cases where there is a fat controller, otherwise, it is better to categorize actions based on their output resources.

Velours