Controller Inheritance
Just a brief interlude from the File Management series while I sort out some time to do some benchmarking.
A common pattern I see in submissions and client-applications is repetitive declarations in controllers. There’s a neat and simple solution for this, but given how often Things like this:
class TodoController < ApplicationController
before_filter :login_required
before_filter :handle_iphone_support
before_filter :fetch_todo
around_filter :performance_logging
def index
...
end
end
For a single controller this set of declarations wouldn’t be a problem, but the problem is when all those declarations end up duplicated in several different controllers. Thankfully every controller inherits from ApplicationController so we can do something like this:
class ApplicationController < ActionController::Base
before_filter :login_required
before_filter :handle_iphone_support
around_filter :performance_logging
end
class TodoController < ApplicationController
before_filter :fetch_todo
def index
...
end
end
This is easy, and obvious and most people do that. However what do you do when you have several controllers which don’t need the login_required call? One option is to give up on inheritance and go back to copying-and-pasting the filter declarations. Another option is to selectively opt-out of the parent controller’s filters.
class ApplicationController < ActionController::Base
before_filter :login_required
before_filter :handle_iphone_support
around_filter :performance_logging
end
class SignupController < ApplicationController
skip_before_filter :login_required
end
However if you have several controllers which don’t require logins, you’ll find yourself duplicating the skip_before_filter declarations around. Otherwise filters and declarations which aren’t completely universal still. An approach which solves this nicely is to introduce an abstract parent controller for all your authenticated controllers.
class ApplicationController < ActionController::Base
before_filter :handle_iphone_support
around_filter :performance_logging
end
class AuthenticatedController < ApplicationController
before_filter :login_required
end
class TodoController < AuthenticatedController
before_filter :fetch_todo
def index
...
end
end
This technique doesn’t just work with filter declarations most other declarations such as caching, session and csrf options work as expected too. In addition you can introduce several parent classes as needed such as a PublicController with page caching declarations, an AdminController for your admin panels etc. Inheritance isn’t the solution to every case of duplicate declarations, yet it’s a simple technique that can simplify most uses.
The Rails Way is all about teaching "best practices"
in 
Another thing to consider is to take advantage of Rail’s mixin behaviour and use modules. This way you avoid the annoyances of a hierarchical structure when you likely don’t mean to have one.
Hi Koz,
I’ve been reading your blog for quite some time now, and I’m usually thrilled with the insights that I get from you. Thanks for that!
However, in this article, you seem to make a classic beginner’s mistake: using inheritance just because it gives you reuse of some code, is a dangerous path. You already touch the reason why in the last paragraph: some of your PublicControllers (which have page caching) might need authorization, some maybe not. How are you going to solve that?
I agree with Adam: if your only concern is reuse of code, mixins are a much better way to do so.
If we’re missing something, please let me know. As I said, I’ve been eagerly reading your blog since you switched to Rails (and a bit before that, in fact).
Cheers!
@Adam: I absolutely agree with you, using mixins would be a great fit if your system doesn’t have a simple hierarchical breakdown as I’ve indicated.
@Tom: No, you’ve not missed something, inheritance is often overused. In fact I’ve been so anti-inheritance in the past that pratik asked me if this article was an april fool’s day post that slipped a week.
Using modules in this situation would take a little more work as you’d need to hook into the included method in order to add the declarations. That would work just fine and is probably a better choice a bunch of the time.
However when you have simple hierarchy such as “all admin controllers must do X” then inheritance is a reasonably nice fit. After all you’re extending ApplicationController already. This technique is a nice simple thing that can save some effort, if you’re fighting it or having more than a few parent classes, ditch it
Agreed with Koz, especially when it comes to “login_required”. We found it much more secure to whitelist actions that don’t require login than to blacklist actions that do.
Here’s a nice helper in AuthenticatedSystem: http://pastie.org/439436
To add to that, I often use this for admin sections and typically namespace the controllers and stick them in an /admin/ folder.
class declarations look like this:
class Admin::AccountsController < Admin::AdminController end
Surprisingly, a lot of people don’t know about this technique and it’s very effective when you want to repeat your controllers for the same resources but have completely different controller logic.
For sure. I find myself doing a combination on inheritance on controllers and controller mixins almost solely for this problem. I find a lot of new Rails guys don’t quite understand that a controller is just a class :)
I hate seeing skip_some_filter hidden away all over in a site. Give me one place to make the change, I’l find it ONCE and be a happy coder!