Overlooked Rails Best Practises Pt.2

In my previous post I covered in bit of depth some of the issues relating to the Rails fixtures system. In this post I’ll cover a couple more bad coding smells that make me cringe, along with simple defensive fixes that will hopefully protect you (especially for those who are still novice Ruby/Rails programmers) from hidden bugs arising later in your development cycle.

In this post I will probably follow on where I left off with issues relating to the issues caused by global data. I am going to home in on a couple of very simple gotchas that I don’t often see mentioned very often in the Rails ‘blogsphere’ that can lead to some very hard to trace bugs that only show up much later in the development cycle (or in some cases not until production deployment).

Use Class / Module Variables With Caution

Code such as:

1
2
3
4

class User
  cattr_accessor :some_variable
  self.some_variable = 'Some String'
end

This was a major gotcha for me when I started using Rails. Many Rails programmers will use class and module variables without giving much thought. Because the variables exist within the scope of the related module or class it is easy overlook the fact this data is effect global.

What’s more is that issues created from this sort of code virtually never manifest themselves in development. The Rails development environment large chunks of code on request, and further more generally runs as a single web server instance.

Why is this an issue?

Firstly, the biggest problem relates to what I outlined above, is due to the way the development environment shielding you away from the side effects this will cause. As a result you will end up with bugs in production that magically disappear when you try and debug them when go into development mode. This is a classic example of a ‘Heisenbug’ in the making!

Example Scenario

A typically example where this will manifest is when you have multiple Rails’ instances running, each request is for a given user can be sent to any of the Rails instances running. However each Rails instance is not going to have it’s own class and module level variables. That is if Joe is served by instance A, which sets a class level variable during the request. When Joe makes another request, it might end up being sent to instance B, because instance A is busy. However because each instance has it’s own class level variables, it can not expect the variable it set in instance A to exist in instance B. That is Rail’s should really be treated as a shared nothing architecture.

Further Issues.

In many cases Rails sites often utilise the ability for user’s to login. A common pitfall here is that user-sensitive data is held in any sort of class level variable, you are going to run the risk of leaking user data between different users. This is not nice given the fact that this issue probably will not expose its self even if you are using some sort of functional tests within your project.

Another overlooked issue is the the possibility for memory to ‘leak’. Well, the memory doesn’t as such leak, but any variables referenced from a class level variable will not get picked up by the garbage collector. This applies to all variable data with it’s referenced directly, or indirectly through a chain of several variables. If you have to store data in class or module variables, it’s generally best to try and stick to simple data types, such as strings and numbers. It is always good when deploying an application to use something like monit to keep a watch on memory consumption, not just on Rails app, but also any part of the Rails application that might run in a persistent / daemonised manner (such as a rake task that remains running in the background until explicitly terminated). I have been caught out a couple of times by unchecked memory consumption myself (Mainly because RMagick needs you to call GC.start after doing image related tasks to clean up C-bound API resources)

Beware Of ‘Constants’

Many people don’t seem to understand the constant semantics fully in Ruby. It is very easy when dealing with constants that are not simple data types to suffer from the same issues as I outlined as above.

Example code

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15

class User < ActiveRecord::Base
  SEARCH_DEFAULTS = {
    :disabled => false,
    :suspended => false,
    # More criteria here
    }
  named_scope :some_search, :conditions => SEARCH_DEFAULTS
end

class UsersController < ApplicationController
  def search
    # Ignore the fact we our not using our named scope here
    @users = User.find(:all, :conditions => User::SEARCH_DEFAULTS.update(params))
  end
end

Despite the fact User::SEARCH_DEFAULTS should be constant, it does not mean the contents of the User::SEARCH_DEFAULTS hash table is constant. This is because Ruby’s constants act much like the const type qualifier in C++. That is firstly if you want, you can explicitly override constants in a similar way that C++ can let you use const_cast to remove the ‘const’ from a variable (Interestingly, Rails exploits the undef_const in it’s Dependency / class reloading system). Secondly, and most importantly in this case, is that only the reference to the object is constant, not the object it’s self. That is you can’t make SEARCH_DEFAULTS reference another object. However you can modify the object SEARCH_DEFAULTS points to. This will most likely trip you up when you are not using a simple data type such as a number, symbol, or maybe string. In the above example each search request will modify the User::SEARCH_DEFAULTS hash table. You need to be particularly careful here when using collection data types.

Preventing This.

There is a very simple fix to this. It’s called the freeze method. All we need to do is call freeze after defining the hash table. For example:

1
2
3
4
5

SEARCH_DEFAULTS = {
    :disabled => false,
    :suspended => false,
    # More criteria here
    }.freeze

This will ensure the hash table object is not mutable (modifiable). This therefore means that we need to update the UsersController#search method to call .dup after referencing User::SEARCH_DEFAULTS because the Hash#update method will modify the the receiver.

Summary

I hope this post will help others. As a seasoned Rails developer, I have to admit that even I got caught out a few months ago by one of the issues I outlined in this post. It goes to show how easy it is to overlook something when you are caught off-guard and are not coding in a defensive manor!

In my next post, I will probably cover some smaller more Rails centric issues that have managed to bite me when getting dirty with other peoples code when haven’t coded defensively.

Listening to Alternative:
English Summer Rain - Placebo