December 2009

You are given the task of validating that no two users register with the same name.

In ActiveRecord it can be implemented like this:

validates_uniqueness_of :name

You may now add an index to your database and consider your task done, but it’s also true that you can improve the user experience by using a better error message:

validates_uniqueness_of :name, :message => "The name {{value}} is taken. <a href="/login">Log in</a> instead?"

Did you see the use of {{value}} in there? That’s because ActiveRecord’s DSL for validations operates on a class level, and that means that when you are declaring the validation, you don’t have access to the instance you will be dealing with. Note also that there’s HTML in the error message. It’s nice to point the user in the right direction, and here it only makes sense to send the user to the login page. But HTML in the models is wrong in many levels, not to mention that the URL helpers are not available there.

It would make much more sense to declare the validations at the instance level, something ActiveRecord allows with the validate method. If we had validates_uniqueness_of as an instance method, this would be possible:

def validate
  validates_uniqueness_of :name, :message => "The name #{name} is taken. <a href="/login">Log in</a> instead?"
end

A detail here is that we can build the message with Ruby interpolations instead of the {{value}} macro. The HTML is still there, but I will deal with that in another post.

Code smells in redundant DSLs

Another example of why a class level DSL for validations is a bad code smell:

validates_presence_of :card_number, :if => :paid_with_card?

def paid_with_card?
  payment_type == "card"
end

What’s wrong here? The validations DSL is reimplementing Ruby functionality, but not because it’s an improvement but because it’s operating on the wrong scope. Returning to the instance level approach, this is how it would look:

def validate
  validates_presence_of :card_number if paid_with_card?
end

There are two other examples in the Rails guides, which I reproduce here:

validates_presence_of :surname, :if => "name.nil?"

As you may know, this evaluates the string within the instance. Needless to say, it’s another case where the DSL reinvents the wheel for no good reason. The counter example:

def validate
  validates_presence_of :surname if name.nil?
end

And now the proc approach:

validates_confirmation_of :password,
  :unless => Proc.new { |a| a.password.blank? }

The counter example:

def validate
  validates_confirmation_of :password unless password.blank?
end

As you can see, these solutions are shorter and make the DSL look superfluous. It means that the DSL is unnecessary.

When you count the lines of code of your models, you should take into account how much code you are injecting. Your models could be hundreds of lines shorter if irrelevant DSLs like this one were removed.

{ 14 comments }