The Worst Rails Code

| Comments

I just came back from RailsConf 2008 in Portland. This year was great. There were a lot of exciting developments to talk about, like MagLev, SkyNet, mod_rails and Rails 2.1.

The talks seemed better this year as well. The one I was most looking forward to was from Obie Fernandez, who wrote The Rails Way, published last fall. I can easily say this is the best Rails book published to date (sorry, Pragmatic). It’s packed with useful information, best practices, and real-world code. Obie’s excellent writing style along with contributions from numerous Rails coders make it a great read too. My copy is already showing wear. And at 900+ pages, it’s like a phone book.

Obie’s talk was given to a packed room, despite being scheduled on Sunday morning at 9am. The title of the talk, ”The Worst Rails Code You’ve Ever Seen (and how not to write it yourself)”, discouraged my friends from attending (“sounds depressing”, one said). During the first lightning round, we had seen some pretty bad code proudly presented (to which Ryan Davis publicly expressed his horror).

But the talk was worth getting up for. Through a series of real-world examples, Obie (and co-presenter Rein Henrichs) showed the audience just how bad Rails coding can get. Some of the code was truly appalling, like a 1200+ line app in a single controller (no, really). Other examples looked, well, kind of familiar. Having been involved in several Rails projects myself since 2005, I’ve seen (and written) my share of bad code.

The talk started out with a bit of an elitist air, with the presenters snobbishly laughing at common mistakes. But as the talk wore on, the tone changed and became more helpful. At one point Obie showed some pretty ugly code and then admitted that he had actually written it.

But what really struck me about this talk was how programmers (myself included) often are unaware of “best practices”, or simply don’t understand Rails and Ruby well enough.

Now some of you reading this may think it’s all pretty obvious, basic stuff. But we all write bad code sometimes, even the slickest meta-programmers I know do. Here’s what I took away from the talk:

Common Reasons People Write Bad Code

1) Folks just don’t write tests, and as a result, they are afraid to refactor (and therefore improve) their code. It’s painful and dangerous to refactor without tests.

2) They are too lazy to look up the correct way to do something, either because they don’t like to read and research, or because they assume it will take longer than just “figuring it out”.

3) They Google for a solution to a problem and come across bad or misleading examples on someone’s blog (or a DZone snippet). You just can’t assume that because it’s posted on someone’s blog that it’s correct. In reality, you should be suspicious of posts without comments or attribution. There are a ton of bad snippets and pasties out there.

What Is Bad Code?

It’s so important to understand the Rails framework. Rails already provides us with many best practices and time savers. It’s silly to waste time re-inventing an already solved problem.

Many of the examples from the talk confirmed that people are struggling with parameters, hashes, and passing data between controller actions.

  • Ruby’s Hash class gives us merge and other useful methods, so there’s often no need to iterate over values or put in explicit assignments.
  • The Rails session object is the proper way to pass data between controller actions, and the flash object is there for displaying messages to the user (and let’s not forget flash.now for AJAX actions). Don’t use instance variables to pass messages to the view.
  • If you have validations on parameters in the controller, they should probably be in the model instead (validates_presence_of, etc).
  • Manipulating parameters outside of the context of a model (or other container) feels like PHP. It’s much nicer to call @MyModel.new(params[:my_model]) and then validate. Why muck around with things like @params[person][date][i1], etc?

Other less-obvious suggestions to help clean up bad code included:

  • Use attr_protected in models instead of worrying about how certain parameters might be set via the url or hacked forms
  • Avoid creating a big config class. Instead, use constants, and put them in the classes that need them.
  • application.rb should only be for actions, put utility methods somewhere else. Like a module, or in models.
  • Consider using the presenter pattern to clean up code

Where To Turn For Help

  • The Ruby Way, 2nd ed. (Hal Fulton) and The Rails Way. They are both excellent.
  • Refer to the Rails API and Ruby standard library documentation as a primary resource
  • When you are having trouble understanding how to use a plugin, Gem, or Rails itself, don’t be afraid to look at the source code. It’s Ruby after all, and often surprisingly easy to understand.
  • Search (or post) on an appropriate IRC channel or forum
  • Study other people’s code. I learn a lot this way. Some of my favorites: Rick Olson, Ryan Davis, Eric Hodel and Evan Weaver.
  • Use script/console: this is often a really fast way to try a concept out, play with it and verify that it will really work.
  • Keep a library of your own code snippets. When you run across something useful, copy and paste it into a file locally that you can search through later.

Don’t Code Solo

Obie also strongly recommended pair programming. Working with other programmers is a really great way to learn quickly. He specifically suggested pairing with a senior coder (like, a Smalltalk guru). For most of this, that’s not a realistic option, unless you are lucky enough to have expert contacts who are willing to spend that kind of time with you. But I can certainly agree that it’s so helpful to work through code with another person.

To that extent, I also found the talk about Remote Pair Programing interesting, especially since I spend a lot of time in Europe while working on projects in the US. I definitely plan to pair program more often now!

Comments