Into the Heart of Darkness: Rails Anti-Patterns (Jake Howerton)

Jake Howerton gave a GoRuCo talk about Rails Anti-Patterns, drawn from his years as a rails developer.

Over lunch with Jake, I’d wondered aloud “where are all the wide-eyed optimistic presentations?” and Jake starts by saying he’s sorry that this will not be one of those talks.

We’ve been programming in Rails for several years now, and now more than ever we’re left with the problem of how to deal with, maintain and correct projects which may be riddled with out-dated thinking, mistaken ideas and problematic implementations.

In other skilled enterprises there are core ideas which are repeated by for practice and for their general utility. In martial arts, these are called “katas,” in programming, we have “patterns.” Patterns are general, reusable solutions to common problems in software engineering, which are often arrived at through emergent design. Anti-patterns, likewise, emerge in the general work on a project, but their presence is harmful. They’re the mistakes we make again and again in our projects.

Jake then laid out a few patterns and anti-patterns for your consideration:

1. Know your APIs

Some of the most egregious anti-patterns come from simple ignorance about the APIs available, and the consequential re-implementation of basic Ruby or Rails functionality. The Rails and Ruby APIs are both well-documented and important to your work. There’s no excuse not to know them.

2. script/plugin and gem installitus

While we must install a plugin or gem to use it, often we move on without doing the important job of cleaning up after ourselves.

3. Style

if not self.value.nil?

People litter ‘self’ all over the place. The only place you need self is on assignment. And all this could be replaced with the simple line:

if value

@value = 600 if @value.nil?

@value ||= 600

some_value == nil?

What does this even mean? ‘nil?’ is false unless inside NilClass

IRB-driven Design

Occasionally we hack away in IRB and, once we get the results we’re looking for, drop the code into our app without a second thought. This can produce some very buggy, obtuse, far-from-minimal code. Once we discover a mechanism, better to revisit the problem and produce the code in a more stable context.

!!value

Confusing. Not to be used outside of a predicate method.

4. Planning is Hard

Will you remember what your boss tells you to do a year ago? Will you unknowingly violate these past decisions and understandings? To avoid doing so it’s important to maintain active specs against your app.

Jake recommends Cucumber + Webrat for carrying this out.

5. How Bad Is It?

There’s a gem called metric_fu, a gem full of analytics. One in particular is known as Reek, which sniffs out code smells.

Run reek against your app, and it will find long methods, for example, and return all of the methods that are longer than 5 lines. And you can make your tests fail on the existence of these methods, to keep yourself and your team in check.

6. Orphans

Orphans are methods, models, views or controllers, or any other bit of functionality which is disconnected from the rest of your app. Like un-used plugins or gems, orphan code is noise which keeps your code from being easily maintainable.

7. (Dis)Organization

We can look to the more modular, merb-style project organization as a way of better structuring our projects. Likewise the Fat-model, Thin-controller design pattern helps us to create easily-testable, well-organized apps.

8. Testing!

The valueless test

assert_equal 2, User.count

This test fixtures, and activerecord, and its very presence hinders us by interfering with our view of more meaningful tests.

The slow test

Any slow test has a cost of both slowing the testing feedback loop and making it less likely that we’ll run the tests as often as we should.

The coupled test

Coupled tests mean that a test depends on the side-effects of other tests, or on the effects of an outside service or application. Instead we can mock out the outside web service or operation

Complex tests mean complex code

9. Make Decisions and Document Them

Discuss your code conventions and style with the team, settle on a set of best practices, and write them down. Store them in a style guide in the project root or document directory.

10. Continuous Integration

Jake recommends using Integrity Continuous Integration Server. Whether you use that or Cruise Control, or any other option, continuos integration insures your specs and tests are frequently and consistently run.

11. Build Artifacts

Generating and maintaining build artifacts, such as statistics reports from your reek run or performance tests can enable you to track your progress over time. In addition, you put minimal expectations on these

12. Reviews and Retrospectives

Taking time every few months to review your recent progress, and discuss problems and progress.

Q & A

Someone mentions a particularly egregious example of a test which deleted its own application code in the process of running.

Jake responds that in cases where tmp deletion and file generation, it may be appropriate to have a safe version of the test which runs locally, and a separate exhaustive test which runs in a sandbox staging environment. This allows you to exercise the more dangerous tests in a safe environment.

Yehuda mentions one way to do this is to mock in the safe version, and turn of that mock in the sandbox environment.

Egregious Code

Jake then shows a 45-line login method and points out how difficult it is to immediately grok. Newer-code built in the REST-ful style is much simpler to work with.

Conclusion

By discussing these approaches with old-hands and newbies alike, we can all improve the quality of our code, spend less time dealing with bugs or code comprehension, and more time building out the next great Rails projects.