The Fat Controller

My latest project is built using the Castle Project stack (Monorail, Windsor, and Active Record). We’re using ActiveRecordMediator so that we can use a Repository pattern with Active Record. We are also using JQuery as our client side Javascript library. At the time we started, some months ago now, we felt ASP.NET MVC was too early in its cycle to use (though we would hope migration would not be too hard if desired). We opted for NHibernate over LINQ To SQL because we wanted to support a fine-grained object model (we are using Domain Driven Design) and because we liked the options around performance and caching that were available with NHibernate. We rejected Entity Framework as unsuitable for our needs (lack of support for test-first development, lack of persistence ignorance, over-complex mapping, poor support for lazy loading etc.)

All that is really just background to this post.The key point is that we are building a MVC web application. As a result of that  experience I wanted to talk about one of the anti-patterns we find ourselves encountering.

The Fat Controller

It’s a big project and we are beginning to get the whiff of where you can give yourself pain using an MVC frameworks. The big one for us is the issue of the ‘fat’ controller. it’s pretty easy to recognize the stink of the fat controller smell. The controller is hundreds of lines long, packed with actions, and the actions are long swathes of code, not the handful of lines that they should be. The two main causes of the stink seem to be:

  • The controller services too many requests (i.e. has too many actions)
  • The controller has domain logic (i.e. model code has crept into the controller)

Why is this bad (just in case you could not see that it was)? I’ll paraphrase the issues identified by Rebecca Wirfs-Brock and Alan McKean in Object Design here:

  • Controller logic is overly complex
  • Controllers have become dependent on domain concepts
  • Controllers increase coupling between otherwise distinct entities, as they pass information from one entity to another
  • Controllers do the interesting work

All of this can really be summed up as making your code hard to maintain. Complex logic is easy to write but hard to read. Simple code requires more effort to write, but pays dividends to maintainers trying to fix or amend it. Breaking the dependencies between your controller and the domain make your code much more ameneable to change. Need that domain logic from another action or another controller. It should be easy because it lives in the domain. Slap it into your controller and suddenly its harder to get at. Perhaps a new customer needs a different view of the domain. Perhaps you want to expose the logic in that controller as an AJAX call instead of on a postback. Perhaps you want to expose it as a WCF service.easy if lives in the domain, hard if it lives in the controller.

That kind of pain tends to lead to cut & paste and re-use. Cut & paste reuse is the road to hell as there are now multiple versions of the truth.Over time they will deviate and you will have confusion.

The more the controllers know about the domain, the more sticky they make it. Want to change the domain, well now you have to change the controllers too. And maybe those UI templates if your domain model was being directly consumed by the view. And that kind of change is more expensive because your refactoring tools probably don’t read your templates.

Skinny Controllers, Fat Model

For my part, the worst smell comes from the controller becoming dependent on the domain. The controller begins to hold domain logic, holding the knowledge of what is is to ship a cargo, purchase a product, make a payment. The domain objects become aneamic, their only role to hold data The interaction becomes a controller setting properties on the domain objects. The controller becomes vampiric, it grows fat by sucking the life-blood out of your domain, which ends up weak, drained, and anaemic.

This is the good old problem of domain-logic in code-behind that we had in ASP.NET webforms. Indeed Webforms are just another type of controller, a page controller, and switching to an application controller model does not remove the need for us to watch for domain logic creeping into the controller. There can be a tempation to believe that just because the controller is easier to test it is now safe to put domain logic in there. Do not fall into that trap.

Fighting the Flab

Our current solution to this smell is to review our controllers to ensure that they are doing very little work. We push responsibility for co-ordinating between domain objects to domain level services. The services have no state, they just co-ordinate the actions of domain objects, such as getting an instance of an entity from the repository and then calling methods on it. This properly belongs in a service that co-ordinates that part of our domain, not the controller whose job is to co-ordinate between the domain (model) and view. 

I suspect, but have no evidence, that a bottom up (or inside-out) and not top down (or outside-in) strategy to development may also help. If we write the domain entities and value types before we write the co-ordinators and controllers we will tend to find that the functionality tends to cluster at the leaf nodes of our domain model. If we start at the top then we will tend to find ourselves putting more logic into the controllers, because we tend to want to write our tests for something other than just calling a couple of objects on the next, and as yet only stubbed out, layer below. I suspect that some of our worse cases of fat controllers may have come from taking a top-down approach. I suspect this is particularly true with folks who drive from the UI. Of course the downside with bottom up or inside-out approaches is because the unit tests drive over the acceptance tests it is easier to slip in speculative functionality that turns out not to be needed by the unit test.

The Rails Way is Skinny Controller, Fat Model and its a good mantra to pick up as you approach development with .NET MVC applications.

Beware God Controllers

The god controller is really just a variation of the god object anti-pattern. Fat Controllers have become God Controllers when they take responsibility for handling too many requests. The pathological case is a site that has just the one controller which handles all requests from the client. Whereas the WebForms page controller forced us into a controller per page, MVC applications don’t force us down this route. So we have to make sensible decisions about what responsibilites each of our controllers should have.

We began by having one controller per aggregate in our domain model.
That seemed attractive at first because of the affinity between
repositories and aggregates but we have quickly hit overload on our
controllers through that approach.

At the other end we might be think
about one controller per entity. This seems attractive at first and Rails sometimes has a controller per model approach,  but ends up too granular. It also breaks down because we
often have multiple entities on one page.

We could move to having a controller per page. This seems attractive when coming from a webforms background but tends to create too many controllers.Developing with MVC frameworks differs from Webforms in that there is no support for server-side controls. If you want dynamic behavior then you need to support it through Javascript and CSS on the client. When mopving to an MVC application you quickly understand that you need to shift from server side controls to client side controls. There are many advantages to this. Your template can render simple clean, standards compliant, HTML. You get clean seperation of the UI widget code away from the controller.

People who have done Javascript in the past shy away at this point because developing rich interactive applications with Javascript used to be expensive. It required a lot of low-level coding against the DOM. The rise of Javascript libraries like JQuery has revolutionized the development of rich interactive applications because these frameworks dramatically lower the cost of development.

What it means in this context is that your controller can handle the code for more than one page very easily. This is true even if you include the actions that your page will expose to client-side AJAX calls.

 A more productive approach looks to be thinking in terms of user
activities and provide one controller per user activity. By user
activity I mean ordering a product, reviewing an account, checking out,
making a payment. These business transactions seem to be a good
granularity level for controllers.

 “God is dead. God remains dead. And we have killed him.” 

Of course the danger is that the domain service itself becomes vampiric
and drains the life-blood of our domain. Control and co-ordination are roles that many services fulfill. Our controller, the ‘C’ in MVC can be thought of as a presentation layer service that co-ordinates between our view and model. Everything that applies to controllers also applies to any service that do control and co-ordination. The fat controller is a specific but not an exclusive case.

We want to watch for swapping our fat controller
for a fat service when we push our code out of the controller and into
a domain service. The mechanism for fighting the flab remains the same. Figure out what control and co-ordination responsibilties this service has and do not spread it to include control and co-ordination respsonsibilites over other areas. Consider the Single Responsibility Principle as a guide. Break up services to stop
them becoming god objects, and push code down into the
entities and value types at the same time. I find that sniffing
for the Feature Envy
smells helps uncover the places where refactoring will help push logic
out of the domain level service and into the domain objects.

Keeping Fit

Development is a bit like life. It is easy to get flabby. We just need to get lazy and eat too many things that are bad for us. The trouble is that once you become overweight it gets harder and harder to lose that weight. So if you don’t want to end up with controllers so overweight that they require surgical intervention, you need to watch how you code them and refactor mercilessly once you see those fat deposits accumulating.

 

About Ian Cooper

Ian Cooper has over 18 years of experience delivering Microsoft platform solutions in government, healthcare, and finance. During that time he has worked for the DTi, Reuters, Sungard, Misys and Beazley delivering everything from bespoke enterpise solutions to 'shrink-wrapped' products to thousands of customers. Ian is a passionate exponent of the benefits of OO and Agile. He is test-infected and contagious. When he is not writing C# code he is also the and founder of the London .NET user group. http://www.dnug.org.uk
This entry was posted in Uncategorized. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • Prash

    Awesome analogies and what a great post !!!! Ian please write more stuff more frequently ^^/

  • John

    “The controller becomes vampiric, it grows fat by sucking the life-blood out of your domain, which ends up weak, drained, and anaemic.”

    This isn’t an argument. This is using words with a negative connotation without justifying the use of those words.

    Why, exactly, is it a bad idea to have POD-style models and all the domain logic in the controller?

    There might be a reason, but I don’t see it in the article.

  • Vangelis Matos Medina

    Great, i agree with the top-down issue.
    Other cause of fat controller is the lack of OO understanding and the anemic model roots.

  • Mr CSharp

    I’m a little confused over the article about the MVC pattern you’re referring to. The reason why I’m a bit confused is that you’re mixing up the UI layer that uses MVC over the backend application that uses MVC as a pattern for controlling data flow for others to consume. If you’re talking controller on an application level or even at the UI level, yes, these should be fairly simple and all the controller should do is what the name implies – just take a bunch of steps and relay them to the model or the view. These steps or actions should be concise statements and should go against a set of business layer calls, the fewer the better. Most of the implementation should be done in the business layer from the application or the SOA standpoint on the backend. On the UI side the controller may consist of a few UI business flow, but most of the real business rules goes against calling the backend controller.

    I think a lot of people put their focus on the UI piece and all they talk about is architecting the front end rather than really focus on getting things right on the backend. The backend application development is where the heart of the application resides and the UI is just a portal to the everything. Sure the UI sees the front end and it is important to make things usable for the users to interact with the system. However, the UI piece should be light and interchangeable with another interface that may be a desktop app on the Mac, Windows, or a Unix GUI, not just the web. People talk about MVC as a solution to try and simplify the interaction of the model and view, but however, people don’t understand the pattern and start implementing cross patterns that basically bypass the logical constraint. So then, at times, you get a two tier system where the view becomes the controller and it has direct access to the data. With MVC, REST, and some sort of ORM nowadays in software development, it seems that a lot of people probably don’t know what they’re doing and having the UI going right at the heart of the data and letting the UI do all the heavy lifting. Then when the UI changes, it’ll be like a total rewrite of the application.

  • Pingback: Domain Driven Design, DDD pour les intimes | Gerald's Blog

  • http://www.facebook.com/robin.scheper.1 Robin Scheper

    How about you create an area for admins in your project and create specific sections for each administrative purpose?

    For example:

    /admin/products/(index | edit | new) (Admin area, ProductsController)
    /admin/seo/(index | edit | new) (Admin area, SeoController)

    On a sidenote:

    I know this post was from 2008, but i hope its a helpfull comment

  • Pingback: ASP.Net MVC architecture discussion – Thin controller, fat model | The Weekly Bacon

  • Pingback: Tinyweb Series: 1 Getting Started | invalid cast

  • http://wyxylysc.com/ Yavjgdgo

    HGKyWK

  • 5x1llz

    Hello

    Wow, I’ve read through all of this and have yet to find a clear/concrete example of how to assign responsibility to controllers that will KEEP them clean.

    How come this remains so vague?

    i.e. if you have an admin controller, you know will obviously get huge. How do you start parsing ot refactoring or designing to avoid a huge God controller that will become unusable?

    So I guess the question is HOW.

    Help!

    Another great post btw Ian, you do present your idea’s and supporting principles very clearly this is why I’m always on this blog!

  • Joe

    I have been fighting this “fat controller” problem myself before even a piece of code has been written. I am having to make a major update to an application that I manage and I figured it was about time to move from spaghetti to mvc as a way to be more consistent with code and take better advantage of DRY principles.

    Well, here I am in the planning stage of how to convert this app to MVC and I can’t wrap my head around thinking of my “pages” into objects with views. In the page-centric design it lent itself to smaller controllers (i’m saying controller in the sense that business logic was put at the top of a file that contained both business logic and view logic.) because they were very specific to each view. So in a sense you had one controller per view.

    Now with these mvc frameworks, and especially ROR I see apps where the controller has 20 or even more actions in a single controller. I just don’t see how that is a good thing. The response I get when I ask about it is…look at your controller as an object and decide all the things it can do and make those your actions. Well, it just seems to me that as your application life cycle progresses, you will end up with those fat controllers that have 30, 40…or more actions (or methods) in them. So much for smaller and discrete requests!

    After reading your post, I kinda like the way you said it might be a better solution to build your controllers around “user activities” and the examples you gave made sense. It’s then very easy to list all the things your application should do (as an acitivity) and then control who has access to those and so on. Rather than say…OK, I’ve got a “Cart” controller that can have 30 actions performed on it, but this user can only access..these actions, and those users can only access these other actions…pretty soon its like, why do you have all those actions in one “cart” controller when it makes the request footprint larger than it needs to be when more than half the stuff in the controller is not even being used.

    Kinda like creating a “User” controller and putting the following actions in it: (based on controller/action)

    user/login
    user/logout
    user/profile
    user/add
    user/edit
    user/delete
    user/…etc.

    Why would you put, Login and Logout code in the user controller when they would only be used once. Why load that extra code in a request each time you want to use the user object.

    In your explanation, “logging in a user” would be an activity that would warrant it’s own controller.

    Am I on the right track here???

  • http://blog.danielfernandes.net Daniel Fernandes

    @Jeremy:
    I agree that there is a fundamental difference between MVC and MVP. I guess RhysC’s confusion is a reflection that a lot of frameworks around, especially MVC ones, are rarely built as “pure” implementations of said UI patterns.
    For instance, most people are clear with both V and C (beware of the flab as per this post) but the M is a bit of “it depends”.
    On a project I’m working on the M is either a DTO or an entity from the Domain Model loaded from NHibernate hence OSIV because we’re not bothered in implementating custom fetching strategies.
    Daniel

  • http://codebetter.com/blogs/jeremy.miller Jeremy D. Miller

    @RhysC,

    *Im not getting caught up on the semantic of MVC vs MVP*

    That’s not just some boring terminology. MVC is significantly different from MVP in implementation.

  • RhysC

    Hey Ian,
    Are you guys using the “Presenter First” approach? i have found this has kept my views very clean, presenters focused and domain suitably rich.
    *Im not getting caught up on the semantic of MVC vs MVP*

    RhysC

  • http://www.UdiDahan.com Udi Dahan: The Software Simplist

    The one rule that I’ve come up with that prevents services, controllers, or anything that works with the domain model from becoming overweight is this:

    From outside the domain, you can use an ORM to retrieve however many objects you like, however, you get to call only one method on only one object out of all those you retrieved.

    I’ve written a bit about this rule in my blog in this post:

    http://www.udidahan.com/2008/06/25/object-relational-mapping-sucks/

    By following this rule, all domain logic tends to stay in the domain model.

    A corrollary of this rule is that you should rarely (though I’d prefer NEVER) call Save/Update methods on your ORM. All transactional logic can be done via an aggregate root that was previously retrieved.

    The nice thing about these rules is that they’re easily enforcable with tools like NDepend.

  • http://blog.danielfernandes.net Daniel Fernandes

    @Henry:
    As Ian said, User Authentication should definitely be in the Presentation layer as usually you would authenticate a user once.
    User Authorization is trickier though as it’s possible runtime business rules are required.

    To do this you could use AOP for instance to inject “security concerns” on domain level entities.

    MyObject {

    [RequiresRoles("Admin,SuperAdmin")]
    public void DoThis() {

    }

    }

    Some code could be set so that it’s called before your DoThis() gets called.

  • Ian Cooper

    @Henry

    Its an infrastructure layer concern. It’s ok for the presentation layer to call the infrastructure layer directly btw.

  • http://henrylearnstorock.blogspot.com/ Henry Ho

    Which layer do you use to handle User Authentication?

  • Ian Cooper

    @Daniel Factoriesand repositories are definitely part of the domain. Services can be if they deal with business logic (the concept of service is general enough to fit in several layers).

    The implementation of your repository may talk to the infrastructure layer either in the form or the API of an ORM or your own Data Access Layer if a legacy system means you can’t use an ORM). It’s fine to call the infrastructure layer from within the repository. Your hand-rolled layer is just the equivalent of NHIbernate (or whatever 3rd party ORM you use). You should be at least as well as abstracted from it as you are with NHIbernate.

  • http://blog.danielfernandes.net Daniel Fernandes

    Copy&Paste gone mad.
    Previous post was mine.

  • http://blog.danielfernandes.net Ian Cooper

    Ian :
    I understand where you coming from and I will be the first to admit I am confused about DDD.

    I read several times several DDD resources and it’s mentioned many times that Repositories, Factories and Services are here to help the domain and can be called by it. This is not to say though that for instance persistence concerns will leak into the domain or that a service need to show too much of its technical implementation. Repositories purpose is to only give the illusion of an in-memory database and therefore its interface should be left simple, an IRepository.BeginTransaction wouldn’t definitely fit in “that” kind of Repository.
    They don’t necessarily live above it such as in a different assembly, in such case you will immediately push some business logic outside of the domain because the domain won’t be able to use these things.
    In my current project we’re having to use NHibernate to map a model on top of a legacy database which uses composite keys, some parts of those keys are assigned and I wanted to make use of Factories to delegate the construction of such Ids (such as going to the database and get the next value in the sequence) and my idea was to use those factories to create Entities.
    It was then decided to move the factories out of the domain model at a higher level, which means from the domain model we can’t any longer construct objects and that’s been delegated to a higher level service that has access to Factories and Repositories.

    Again, I am very confused about DDD and I am just trying to challenge other people, such as yourself, so that I can get a better opinion of it :)

  • Ian Cooper

    @Daniel

    I would still avoid directly calling a factory or repository from within an entity and push that out to a service call in many circumstances. Co-ordination is the job of services. The problem is that its easy to confuse co-ordination or our abstractions with automation of business process. Having worked on systems that did not cleany seperate these, my experience is that it makes it hard to understand what the consequences of making a call are i.e. I may want to change state without persisting it yet.

    BTW I don’t think that onion, or for my part hexagonal architectures cause aneamic domain models. In fact they work against it. In an hexagonal architecture everything outside the domain is accessed by a port/service or accesses the domain through port service.

    The goal is to seperate technology concerns – widget logic in the presentation layer, wcf clients, etc. from the domain. This makes it easier to get your domain under unit test – because you don’t have techology concerns that you need to replace with test doubles.

    The domain logic is the key area to get under unit test.

    Its great to get my controllers under test so that I can confirm the communication with the view, and put quality checking in at the coding point where the feedback is most cost-effective in preventing defects but the sweet spot in terms of returns is the domain.

    Your UI, DB, services, etc. all need functional testing to confirm the acceptance criteria for your story, and that will pick up issues with the ‘technology’ areas.

    Just remember to do your functional testing within the same iteration as you deliver the story not afterwards.

  • Ian Cooper

    @Derik Agreed, but teams do acquire technical debt even when we do not want too. Incremental re-architecture sometimes means that you defer the refatoring a little too long. It’s a bit like eating donoughts. I know its bad but sometimes it happens…

  • http://s.rottem@symbiotic-development.com Symon Rottem

    Man, your first paragraph is *exactly* the reasons we chose the same stack for the project we’ve been working on. Good to know we’re not alone in our thinking…

    Good post – you’ve nicely and succinctly summed up the issues we’ve been running up against and this is very useful info.

    Cheers,

    Symon.

  • http://blog.danielfernandes.net daniel fernandes

    Excellent post.
    I think a way of making sure that responsibilities that belong to the domain don’t end in controllers or domain/application services is to empower the domain in being able to do so.
    That means domain objects can use of services, repositories and object factories, obviously while still applying SOLID principles. And we can use messaging via an internal event broker to fairly easily extend responsibilities of the domain. Having read the DDD book from InfoQ I believe that a pure Onion architecture can only lead with an anemic domain model.
    And on the particular issue of domain model leakage to UI, I guess a Presentation Model may be the best route to take even if it means a lot of mappers.

    Daniel

  • http://devlicio.us/blogs/derik_whittaker/default.aspx Derik Whittaker

    Well put.

    It worries me to see fat controllers because all they do is increase clutter and decrease efficiency.

    If everyone just follows SOLID principles (I know, easier said than done) than all ‘should’ be well with the world.