Single Action per Controller in ASP.NET MVC

As I keep doing this on MVC projects, thought I may as well blog it, so the next time I need it I can find it myself :)

To keep ASP.NET MVC controllers clean and organised, I find it much easier to split them so they handle only a single action each. This helps significantly with maintaining some semblance of the Single Responsibility Principle, allowing each controller to deal with a single action, and not have concerns like Logon, Logoff and ChangePassword all start globbing together under a single controller called "Account"

With a little bit of magic in the ControllerFactory you can tell ASP.NET MVC3 to resolve controllers from your own directory and class structure, meaning we can have controllers that look like:

public class IndexController : Controller
{
   [Authorize]
   public ActionResult Index()
   {
      return View();
   }
}

https://gist.github.com/1075169

The Controller Factory

I tend to use Castle Windsor to do this magic for me, though any IoC container would do, and you could even do it without a container if you were really masochistic!

In Windsor the ControllerFactory looks like:

  public class WindsorControllerFactory : DefaultControllerFactory
    {
        private readonly IKernel kernel;

        public WindsorControllerFactory(IKernel kernel)
        {
            this.kernel = kernel;
        }

        public override void ReleaseController(IController controller)
        {
            kernel.ReleaseComponent(controller);
        }

        protected override IController GetControllerInstance(RequestContext context, Type controllerType)
        {
            var baseNs = typeof(SecureController).Namespace;

            var ns = context.RouteData.GetRequiredString("controller").ToLower();
            var controllerName = context.RouteData.GetRequiredString("action").ToLower() + "controller";
            try
            {
                var controller = kernel.Resolve<IController>(baseNs.ToLower() + "." + ns + "." + controllerName);
                return controller;
            }
            catch (ComponentNotFoundException ex)
            {
                throw new HttpException(404, string.Format("The controller for path '{0}' could not be found.", context.HttpContext.Request.Path));
            }
        }
    }

https://gist.github.com/1075171

The Controller Installer

So that Windsor has some way of resolving our controllers, you need to register them in the container. This installer will scan for all classes implementing IController and add them into the container against a key matching their namespace:

public void Install(IWindsorContainer container, IConfigurationStore store)
{
     foreach (var type in Assembly.GetExecutingAssembly().GetTypes())
     {
      if (typeof(IController).IsAssignableFrom(type) && !type.IsAbstract)
      container.Register(Component.For(type).ImplementedBy(type).Named(type.FullName.ToLower()).LifeStyle.Transient);
     }
}

https://gist.github.com/1075176

Which leaves your controller folder structure looking like:

GET and POST

Where an action has a Get and a Post action, for example where you display a form, then take some actions based on the form, you just put the two actions in the same Controller, you aren't breaking SRP here as both actions are inherently linked anyway:

   public class ChangePasswordController : Controller
    {
        private readonly IUsersDB users;

        public ChangePasswordController(IUsersDB users)
        {
            this.users = users;
        }

        [Authorize]
        public ActionResult ChangePassword()
        {
            return View();
        }

        [Authorize]
        [HttpPost]
        public ActionResult ChangePassword(ChangePasswordViewModel model)
        {
            if (ModelState.IsValid)
            {
                try
                {
                    users.ChangePassword(User.Identity.Name, model.OldPassword, model.NewPassword);
                    return RedirectToAction("ChangePasswordSuccess");
                }
                catch (DataAccessException ex)
                {
                    ModelState.AddModelError("", "The current password is incorrect or the new password is invalid.");
                }
            }

            return View(model);
        }
    }

Posted 07-11-2011 2:08 AM by Jak Charlton
Filed under: , ,

[Advertisement]

Comments

hazzik wrote re: Single Action per Controller in ASP.NET MVC
on 07-11-2011 1:26 AM

What advantages of this approach?

Jak Charlton wrote re: Single Action per Controller in ASP.NET MVC
on 07-11-2011 1:37 AM

1) You don't end up with massive controller classes

2) Due to the relative verbosity of C#, a controller class is mostly filled with blank lines and braces (controllers should have almost no logic). I find that doing this lets me concentrate easily on the actual functionality and not have to scan a lot of whitespace

3) and most important of all - you don't do things like overloading constructors with a dozen services or repositories to support every operation - you can focus on the single responsibility the controller action really has. SRP FTW :)

Martin wrote re: Single Action per Controller in ASP.NET MVC
on 07-11-2011 5:05 AM

I took a similar approach with the Tinyweb framework I wrote, where a Handler class can have Get/Post/Put/Delete methods only and the handler class itself determines the URL.

From your example, IndexHandler, LogonHandler and LogoffHandler would produce /index, /logon and /logoff respectively and force the pattern you mention.

You can find the project here if you're interested: github.com/.../Tinyweb

Mike wrote re: Single Action per Controller in ASP.NET MVC
on 07-11-2011 5:25 AM

is this smart, or just plain trickery?

1) Use a single .cs file with separate controller classes

2) Use a single .cs file with a single class with several nested controller classes.

I experimented with each but it's more about code organisation...

Jon Galloway wrote re: Single Action per Controller in ASP.NET MVC
on 07-11-2011 11:57 AM

While it doesn't help at all with SRP issues (cases of overly complex controllers and constructor overload abuse), if you want to split up actions among multiple files for readability you can just split a controller into partial classes.

jrnail23 wrote re: Single Action per Controller in ASP.NET MVC
on 07-11-2011 12:10 PM

Hi Jak, good to see someone else exploring this approach as well.  Here's a link to Derek Greer's discussion of single action controllers, which also asks and answers some interesting questions: lostechies.com/.../single-action-controllers-with-asp-net-mvc

tobi wrote re: Single Action per Controller in ASP.NET MVC
on 07-11-2011 8:42 PM

1) and 2) can be solved by training - be pragmatic, not obsessive. 3) is harder. I solved it by using a base class receiving a bunch of common services by property injection and extending the action invoker to inject dependencies as method parameters.

Quooston wrote re: Single Action per Controller in ASP.NET MVC
on 07-12-2011 3:53 AM

+1 for partial classes for better organization.

Jak Charlton wrote re: Single Action per Controller in ASP.NET MVC
on 07-12-2011 7:49 PM

Partial classes should only ever be used by code generation

Splitting code arbitrarily across files just to increase readability is masking another problem and creating more problems - if it's too long/complex to read easily in one file, it's probably breaking SoC, SRP and a whole bunch of other good practices

But mostly, this is about splitting responsibility - there should be only one reason for a class to change - and a controller with even fairly closely related methods has many reasons for change

Quooston wrote re: Single Action per Controller in ASP.NET MVC
on 07-14-2011 6:58 AM

I agree, using partial classes in order to disguise bigger problems related to SRP and such is not a good thing. I don't believe one action per controller is a good thing either.

I would say that if you can't handle a Login, and a Logout action in one controller based on an overzealous SRP gland, you're just not being pragmatic enough.

Also, often times in something like a repository class, partials come in handy all the time; and not due to infrastructure requirements either. Linq statements can become very verbose and sometimes it makes a lot of sense to split the class into partials purely for the sake of readability. And no, creating another repo just because of a verbose syntax, citing SRP as a reason is not a good answer there either.

About The CodeBetter.Com Blog Network
CodeBetter.Com FAQ

Our Mission

Advertisers should contact Brendan

Subscribe
Google Reader or Homepage

del.icio.us CodeBetter.com Latest Items
Add to My Yahoo!
Subscribe with Bloglines
Subscribe in NewsGator Online
Subscribe with myFeedster
Add to My AOL
Furl CodeBetter.com Latest Items
Subscribe in Rojo

Member Projects
DimeCasts.Net - Derik Whittaker

Friends of Devlicio.us
Red-Gate Tools For SQL and .NET

NDepend

SlickEdit
 
SmartInspect .NET Logging
NGEDIT: ViEmu and Codekana
LiteAccounting.Com
DevExpress
Fixx
NHibernate Profiler
Unfuddle
Balsamiq Mockups
Scrumy
JetBrains - ReSharper
Umbraco
NServiceBus
RavenDb
Web Sequence Diagrams
Ducksboard<-- NEW Friend!

 



Site Copyright © 2007 CodeBetter.Com
Content Copyright Individual Bloggers

 

Community Server (Commercial Edition)