This post originated from an RSS feed registered with Ruby Buzz
by Chad Fowler.
Original Post: 20 Rails Development No-No's
Feed Title: ChadFowler.com
Feed URL: http://feeds2.feedburner.com/Chadfowlercom
Feed Description: Best practices, worst practices, and some generally obvious stuff about programming.
Rails programmers: what's an example of one thing you find in other people's Rails
code that you (almost) always consider to be wrong?
Quite a few people responded, and several asked if I’d post the aggregated answers. Here’s an attempt. I don’t necessarily agree with everything here. I’d love to hear more of these in the comments.
@briandoll’s response was particularly interesting:
extensive logic in views. If the erb would be ugly and harder to pull off in
haml, it's likely a code smell
This is something I always liked in Java about XMLC:http://www.enhydra.org/tech/xmlc/index.html. It was really hard to end up with a mess of co-mingling code and views, because you would have to work harder in XMLC to even make that possible. Amrita used to have that same characteristic, but I’m not sure it still does (or if it is even maintained).
I’ve been resistant to HAML but maybe it’s time to give it another look.
Also, as @mbleigh points out, any code in views is suspect. CSS should be in CSS files. Javascript should be in Javascript files.
@seancribbs singles out auto-generated tests. I’ll have to agree with him.
@alancfrancis says simply “rspec”. Certainly not a religious topic. I’m sure if you could somehow see in the code which editor someone used, we’d see a lot of TextMate, vi, and emacs in the responses.
4. Bad use of Polymorphic Associations and Single-Table Inheritance
@brendanbaldwin says “I’ve seen Polymorphic Associations and Single Table Inheritance abused to absurdity.”
As have I. I’ve also abused them to absurdity myself. The storage of a column called “type” which holds a class name is a pretty good indicator that something fishy is going on. It’s fishy but not always bad. I think, though, that any time you use it you should ask yourself more than once if it’s the right solution. Databases don’t do what they do best as well when you have lots of STI and polymorphic associations.
I once had to debug a problem with a commercial, Java-based portal product. The mail module was failing, saying it couldn’t connect to the mail server. In fact, what was happening was my login credentials were incorrect. Guess what the offending JSP looked like (pseudo-code)?
try {
// do everything here
} catch (Exception e) {
out.println("Unable to connect to the mail server") ;
}
Exceptions are one of the only places in Ruby where you rely on checking the classes of objects to do useful and interesting things. Never rescue Exception unless it’s at the end of a stanza in which you’ve already handled one or more other exception types.
validates_uniqueness_of is a strange critter. It’s so easy to use, that it’s tempting to slap it into your code and consider it good enough. But even the documentation for it warns you:
Using this validation method in conjunction with ActiveRecord::Base#save
does not guarantee the absence of duplicate record insertions, because
uniqueness checks on the application level are inherently prone to race
conditions.
OK, so this isn’t really what he meant. But it’s a good thing to think about. If you find yourself writing a lot of code to do something simple, you might be missing something Rails (or a plugin) provides. I’ve seen a whole lot of wheels re-invented inside models, controllers, and helpers. Part of Rails mastery is learning to ask yourself, “Shouldn’t somebody have already done this for me by now?”
Seems like if you find yourself manipulating Ruby’s load path in your Rails code, you’re doing something strange. If you don’t know you’re doing something strange, then you’re probably doing something wrong.
As Dave Thomas likes to say “Don’t not use positive logic” (or something like that).
Can someone tell me where the use of ”!!true” crept into the Ruby vernacular? Sure, it’s syntactically valid to do:
!!!!!!!!!!!!!!!!!!ok?
But why?
It’s April 1st, so my mind wanders to the hypothesis that someone introduced this and started spreading it as an elaborate joke. If so, well played. Is there a good use of this idiom I’m missing?
ActiveRecord migrations define a simple API/DSL for doing schema manipulation. But when they run, they’re just Ruby classes and methods executing within the Rails environment. So you can do anything you like in that context.
Sometimes people use migrations to load seed data. Pratik doesn’t like this.
I’m on the fence on this one. In small doses it doesn’t strike me as a horrible practice. That being said, I don’t tend to do it. I’d be interested in hearing opinions.
I guess it depends on what you’re doing with the parameters. Sometimes long parameter lists on GET requests are a sign that there are two actions’ worth of work happening in a single action. Or that you’re doing something that belongs behind a POST or PUT. This one doesn’t bother me as much as it does @Adkron.
Smalltalk people, as a collective culture, seem to get this right. Kent Beck documents a simple pattern in his Smalltalk Best Practice Patterns called “Compose Method”. He says that one way to determine whether a new method is required is whether all of the code in a given method operates at the same level of abstraction. That’s a simple but powerful rule. Think about it next time you’re coding and I think you’ll agree.
I agree with Mike on this one. I don’t tend to write any comments, so I may be on the anti-comment side to a fault. My goal is always to write code that is clear enough that comments don’t add anything. If you name everything well and keep your classes, modules, and methods short, you’re already going to make comments redundant in a lot of cases.
Whenever I see a comment inside a method definition, I consider it a bad sign. In most cases, whenever you encounter a comment inside a method, it’s a marker for where code should be extracted into a new method.
If you’re writing a framework, the rules are different. But most of the time as a Rails developer, you shouldn’t be writing a framework.
Previous Topic
