Five Common Rails Mistakes

No Comments

I’ve worked with Rails for quite a while now and in that time I’ve seen a lot of Rails applications and both read and written a lot of bad Ruby code. Here’s five common mistakes that I see in almost every Rails codebase.

• Migrations with no schema specifics

Your data model is the core of your application. Without schema constraints, your data will slowly corrode due to bugs in your codebase until you can’t depend on any fields being populated. Here’s a Contact schema:

create\_table “contacts” do |t| t.integer “user\_id” t.string “name” t.string “phone” t.string “email” end What is required? Presumably a Contact must belong_to a User and have a name — use database constraints to guarantee this. By adding :null => false, we ensure that the model is always consistent even if we have bugs in our validation because the database will not allow a model to be saved if it fails those constraints.

create\_table “contacts” do |t| t.integer “user\_id”, :null => false t.string “name”, :null => false t.string “phone” t.string “email” end Bonus points: use :limit => N to size your string columns appropriately. Strings default to 255 characters and phone probably doesn’t need to be that big, does it?

• Object-Oriented Programming

Most Rails developers do not write object-oriented Ruby code. They write MVC-oriented Ruby code by putting models and controllers in the expected locations. Most will add utility modules with class-methods in lib/, but that’s it. It takes 2-3 years before developers realize: “Rails is just Ruby. I can create simple objects and compose them in ways that Rails does not explicitly endorse!”

Bonus points: introduce facades for any 3rd-party services you call. Provide a mock facade for use in your tests so that you don’t actually call the 3rd party service in your test suite.

• Concatenating HTML in helpers

If you are creating helper methods, kudos, at least you trying to keep your view layer clean. But developers often don’t know the basics of creating tags within helpers, leading to messy string concatenation or interpolation:

str = “

” str.html\_safe Yikes, it’s ugly and can easily lead to XSS security holes! content\_tag is your friend.

content\_tag :li, :class => ‘vehicle\_list’ do link\_to(“#{vehicle.title.upcase} Sale”, show\_all\_styles\_path(vehicle.id, vehicle.url_title)) end Bonus points: start introducing helper methods that take blocks. Nested blocks are a natural fit when generating nested HTML.

• Giant queries loading everything into memory

You need to fix some data so you’ll just iterate through it all and fix it, right?

User.has\_purchased(true).each do |customer| customer.grant\_role(:customer) end You have an ecommerce site with a million customers. Let’s say each User object takes 500 bytes. This code will take 500MB of memory at runtime! Better:

User.has\_purchased(true).find\_each do |customer| customer.grant\_role(:customer) end find\_each uses find\_in\_batches to pull in 1000 records at a time, dramatically lowering the runtime memory requirements.

Bonus points: use update_all or raw SQL to perform the mass update. SQL takes time to learn well but the benefits are even more tremendous: you’ll see a 100x improvement in the performance.

• Code review!

I’m guessing you are using GitHub and I’m also guessing you aren’t using pull requests. If you spend a day or two building a feature, do it on a branch and send a pull request. Your team will be able to review your code, offer suggestions for improvement and possible edge cases that you didn’t consider. I guarantee your code will be higher quality for it. We’ve switched to using pull requests for 90% of our changes at TheClymb and it’s been a 100% positive experience.

Bonus points: Don’t merge pull requests without tests for at least the happy path. Testing is invaluable to keep your application stable and your sleep peaceful.

Did I miss any common issues? Leave a comment and let me know!

Update: use find\_each rather than find\_in_batches, thanks readers!

Original Post:

http://www.mikeperham.com/2012/05/05/five-common-rails-mistakes/