views:

138

answers:

3

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?

+8  A: 

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.

Patrick Karcher
Thank you for the well thought out answer - very helpful.
Mayo
+2  A: 

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.

Dave Swersky
+1  A: 

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.

Sam Saffron
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?
Ryan