Ian Cooper

Sponsors

The Lounge

Advertisement

Images in this post missing? We recently lost them in a site migration. We're working to restore these as you read this. Should you need an image in an emergency, please contact us at imagehelp@codebetter.com
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.

 


Posted Wed, Dec 3 2008 8:25 AM by Ian Cooper

[Advertisement]

Comments

Derik Whittaker wrote re: The Fat Controller
on Wed, Dec 3 2008 8:43 AM

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.

daniel fernandes wrote re: The Fat Controller
on Wed, Dec 3 2008 3:58 PM

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

Symon Rottem wrote re: The Fat Controller
on Wed, Dec 3 2008 4:16 PM

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.

ASP.NET MVC Archived Blog Posts, Page 1 wrote ASP.NET MVC Archived Blog Posts, Page 1
on Thu, Dec 4 2008 12:41 AM

Pingback from  ASP.NET MVC Archived Blog Posts, Page 1

Reflective Perspective - Chris Alcock » The Morning Brew #237 wrote Reflective Perspective - Chris Alcock » The Morning Brew #237
on Thu, Dec 4 2008 5:23 AM

Pingback from  Reflective Perspective - Chris Alcock  » The Morning Brew #237

Ian Cooper wrote re: The Fat Controller
on Thu, Dec 4 2008 6:21 AM

@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...

Ian Cooper wrote re: The Fat Controller
on Thu, Dec 4 2008 6:44 AM

@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 wrote re: The Fat Controller
on Thu, Dec 4 2008 8:27 AM

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 :)

Dew Drop - December 4, 2008 | Alvin Ashcraft's Morning Dew wrote Dew Drop - December 4, 2008 | Alvin Ashcraft's Morning Dew
on Thu, Dec 4 2008 9:07 AM

Pingback from  Dew Drop - December 4, 2008 | Alvin Ashcraft's Morning Dew

Daniel Fernandes wrote re: The Fat Controller
on Thu, Dec 4 2008 9:09 AM

Copy&Paste gone mad.

Previous post was mine.

Ian Cooper wrote re: The Fat Controller
on Thu, Dec 4 2008 9:23 AM

@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.

Henry Ho wrote re: The Fat Controller
on Thu, Dec 4 2008 5:10 PM

Which layer do you use to handle User Authentication?

Ian Cooper wrote re: The Fat Controller
on Fri, Dec 5 2008 4:39 AM

@Henry

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

Daniel Fernandes wrote re: The Fat Controller
on Fri, Dec 5 2008 5:55 AM

@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.

Udi Dahan: The Software Simplist wrote re: The Fat Controller
on Mon, Dec 8 2008 8:29 AM

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:

www.udidahan.com/.../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.

Code Monkey Labs wrote Weekly Web Nuggets #41
on Mon, Dec 8 2008 11:48 AM

Pick of the week: How I Put My Wife Into SVN General What Makes a Good Unit Test : Mark Needham has some good pointers on making your unit tests shine. The Problem With Logging : Jeff Atwood shows how easy it is to abuse logging and what the repercussions

Steve Strong's Blog wrote Weekly digest of interesting stuff
on Wed, Dec 10 2008 4:17 PM

Weekly digest of interesting stuff

RhysC wrote re: The Fat Controller
on Fri, Dec 12 2008 8:09 AM

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

Jeremy D. Miller wrote re: The Fat Controller
on Fri, Dec 12 2008 10:32 AM

@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.

Daniel Fernandes wrote re: The Fat Controller
on Fri, Dec 12 2008 11:22 AM

@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

DotNetKicks.com wrote The Fat Controller
on Mon, Dec 29 2008 8:18 AM

You've been kicked (a good thing) - Trackback from DotNetKicks.com

Joe wrote re: The Fat Controller
on Thu, Jan 8 2009 2:42 PM

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???

Ian Cooper [MVP] wrote "Inappropriate Intimacy" points to the bleeding out of your domain logic
on Thu, Feb 19 2009 4:25 AM

A while ago I posted about Fat Controllers . Another area that can quickly get beyond you are any domain

Community Blogs wrote "Inappropriate Intimacy" points to the bleeding out of your domain logic
on Thu, Feb 19 2009 4:41 AM

A while ago I posted about Fat Controllers . Another area that can quickly get beyond you are any domain

Ian Cooper [MVP] wrote With Services the focus is on behavior
on Sat, Feb 21 2009 11:09 AM

This post is really just tying together some ideas. A year ago I posted on my preference for classicist

Community Blogs wrote With Services the focus is on behavior
on Sat, Feb 21 2009 11:28 AM

This post is really just tying together some ideas. A year ago I posted on my preference for classicist

Code Monkey Labs wrote Weekly Web Nuggets #41
on Sun, Feb 22 2009 10:34 PM

Pick of the week: How I Put My Wife Into SVN General What Makes a Good Unit Test : Mark Needham has some good pointers on making your unit tests shine. The Problem With Logging : Jeff Atwood shows how easy it is to abuse logging and what the repercussions

Coding: Making the debugger redundant at Mark Needham wrote Coding: Making the debugger redundant at Mark Needham
on Sun, Mar 22 2009 5:54 AM

Pingback from  Coding: Making the debugger redundant at Mark Needham

Kazi Manzur Rashid's Blog wrote ASP.NET MVC Best Practices (Part 1)
on Wed, Apr 1 2009 11:45 AM

In this post, I will share some of the best practices/guideline in developing ASP.NET MVC applications

5x1llz wrote re: The Fat Controller
on Mon, May 11 2009 5:19 AM

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!

Yavjgdgo wrote re: The Fat Controller
on Tue, Jul 14 2009 8:43 AM

HGKyWK

dorah wrote The Fat Controller must die!
on Fri, Nov 13 2009 8:50 AM

As most parents with little boys know the Fat Controller is a key figure in the Thomas the Tank Engine...

Add a Comment

(required)  
(optional)
(required)  
Remember Me?