Mark Needham

One thing I’ve noticed while browsing our Rails code base is that the first entry point inside a controller is much less frequently the method corresponding to the action than it would be with a C# ASP.NET MVC application.

The concept of filters exists in ASP.NET MVC but on the projects I’ve worked on they’ve been used significantly less than before filters would be in a Rails application.

As a result I’m getting much more in the habit of checking for the before filters in the ApplicationController when an action isn’t working as expected to try and figure out what’s going on.

One which caught us out the other day is the ‘rescue_from’ method which gets mixed in from Rescuable.

Every single time we tried to login we were just getting redirected back to the login page again with no exception being reported anywhere.

Eventually we realised that the InvalidAuthenticityToken exception was being thrown because we somehow had a different token stored in our browser’s cookie than was in the form.

Since our LoginController extends the ApplicationController, the following logic was being used to handle the exception:

class ApplicationController < ActionController::Base
   rescue_from ActionController::InvalidAuthenticityToken, :with => :redirect_to_referer
 
  def redirect_to_referer_or_path
    redirect_to request.referer
  end

We initially didn’t spot it because it was hidden away between a whole load of other before filters and includes of modules.

The convention as far as I can tell is to try and keep as little code as possible inside the action method and handle common logic – such as looking up a user based on an id or handling exceptions – in filters.

It helps to keep the code inside actions pretty clean and remove duplication but I just need to remember to check both the action and the top of the class for any filters when investigating problems.

I just came across a really cool blog post written a couple of months ago by Evan Light where he proposes that we ‘write the first one ugly‘:

To overcome paralysis, for small chunks of code, it is often better to just write whatever comes to mind – no matter how awful it may seem at the time. Give yourself permission to let the first version suck.

I think this is a really good piece of advice and it seems along the same lines as a suggestion from Uncle Bob in Clean Code:

When I write functions they come out long and complicated…then I massage and refine that code, splitting out functions, changing names and eliminating duplication…all the whole keeping the tests passing.

I find myself following a similar approach to Evan these days whereas previously I’d probably have spent a lot of time thinking through the problem in my head trying to come up with a design I was happy with.

I agree with Evan that it’s frequently easier to see a clean way to solve a problem if you actually have some sort of code in front of you even if it is a terrible solution.

If I get stuck I tend to copy and paste other code, break encapsulation of objects, write long methods and so on. Then after I have something actually working I’ll go back and clean it up.

Sometimes I don’t see a way to write a piece of code more cleanly so I’ll leave it alone until the next time we’re working in that area of the code base, an approach that Joshua Kerievsky expands upon in a post he wrote a few months ago titled ‘sufficient design‘.

I’ll leave the final word to Simon Harris who made the following observation on twitter which I think is pretty accurate:

Ive said it before the difference between great developers and hacks is: the former clean up after themselves.