Sunday, December 24, 2006

after_filter in Rails -- pitfall

In Rails, as far as I can tell, if you've got an instance variable that needs to be set the same way every time, using after_filter to set it won't work. This probably isn't a bug; it makes sense because filters are what you filter your actions through. You use before_filter to make sure your users are logged in, for instance.

The reason I'm thinking about this is that I recently came across some code where somebody had used before_filter to set an instance variable. I thought, hmm, that's not what you're supposed to do, but does it work for after_filter? Can I misuse that the same way? It turns out the answer is no, but it makes me wonder, if the fact that you can't misuse after_filter that way is a feature, then surely the fact that you can misuse before_filter that way is a bug.

It's debatable, though. It isn't really a problem in the framework; it's just a design flaw, because it allows you to do something like this:

before_filter :foo

def foo
  @user = User.find(session[:user])
end


Where you're no longer really filtering, you're just doing something first. The thing is, Rails is very, very easy to use, and this attracts people who like simple tools. This means it both attracts people who love the elegance of simple tools, and people who aren't prepared to wrestle with complex tools. But the distinction between filtering and merely doing something first is a subtle distinction. Having to be consciously aware of it is somewhat inelegant, and people who aren't prepared to use complicated tools are probably going to trip over this distinction at least once. I did, and I can usually handle complicated tools without any problem.

Anyway, it threw me for a second that you can misuse the one filter but you can't misuse the other. But the explanation is pretty simple. You can't misuse after_filter because the actions render in the actions. The after_filter is called after all the rendering happens, so even though you are setting an instance variable on the controller, it won't matter, because the instance vars were already copied to the view before you even got there. The comments in Rails depict it as a chain:

# Given these filters
# after_filter :after
# around_filter :around
# before_filter :before
#
# The filter chain will look like:
#
# ...
# . \
# . #around (code before yield)
# . . \
# . . #before (actual filter code is run)
# . . . \
# . . . execute controller action
# . . . /
# . . ...
# . . /
# . #around (code after yield)
# . /
# #after (actual filter code is run)
#
(Weirdly, I could only find these comments in the 1.2 release candidate, but I could swear I've seen them before.)

Anyway -- this is kind of a subtle pitfall for newbies. Actually, screw newbies, it's a subtle pitfall for me. I fell into it, and I've been making Web apps for a decade. They even called me a Perl guru at one point. Anyway, yay my ego. Point is, subtle pitfall. There are two parts to it. First, just because you can use before_filter as a generic preprocessor doesn't mean you can use after_filter as a generic postprocessor, and second, just because you can do the preprocessor thing doesn't actually make it a good idea. It is in fact a bad idea.

Moral of the story: only use before_filter for filtering.

(I had a look at the Rails source to see if I could find a way to patch this, but it was just a quick glance, and no dice so far.)

By the way, one interesting footnote to this experience. There's a very smart man who says that good programmers avoid working on stupid problems for the same reason models avoid cheeseburgers. The general idea is that working on bad code makes you a bad coder. For a while I agreed, but now I'm not so sure. I was working on bad code here, when I encountered this anti-pattern, and it wasn't until I copied the mistake in a different context -- applying a before_filter misuse to an after_filter -- that I realized it was a problem at all. On the one hand, this supports the idea that working with bad code makes you a bad coder, because clearly this was a case of monkey see, monkey do. But puzzling over my mistake led me to examine the Rails source and consider a relatively subtle design question. It's entirely possible that working with bad code made me a better programmer, in this instance.

It reminds me of when I was a kid, we had a Salvadoran housekeeper. She was a refugee from Reagan's secret wars in Central America. She cleaned toilets for a living, and she had a master's degree. Her husband had a doctorate. My mother asked her, why don't you get transcripts from the university? But the university had been bombed to smithereens, and not only were her transcripts gone, her professors were all dead. So my mother thought this woman must be hating her life, cleaning toilets when she had an advanced degree, but in fact she was just happy she was still alive. She said, it's not the job, it's how you do it.

The reverse is also true. I've seen people working on fascinating problems who made themselves worse coders in the process just because they were approaching interesting questions in a stupid way. I've done it myself; the most interesting piece of software I ever built was for an idealistic startup with great ambitions but no clue on earth how to run a software business. I reviewed the code later and was flat-out embarassed about how dumb some of the mistakes I made were. The problem came from being perpetually distracted by business issues that had nothing to do with the software. Approaching a set of interesting questions in a stupid way, I made stupid mistakes.

So, it's possible Paul Graham might actually be wrong about something (gasp!).

2 comments:

  1. ok this all makes sense - but what would be the best practice in a case where all actions need the same additional work?
    (i need a list of all articles no matter what, and i need it processed after the main action like insert/update/delete and also new/edit)
    And thanks for the article it made me see the point i would have overlooked.

    ReplyDelete
  2. As far as I can tell, the official answer is that before_filter is the best practice for invoking a given set of code that's common to all actions in a controller. But I think that answer's wrong, or at least creates the vulnerability to this pitfall. I don't know. I'm going to look into it.

    ReplyDelete

Note: Only a member of this blog may post a comment.