Zuul and Code Etiquette

Obie and a few others tweeted about Zuul earlier today - a plugin for adding ‘role-based authentication’ to Rails. I had a quick look at the code to see it’s particular take on this common problem, but was struck by an unfortunate aspect of it’s current implementation.

When you requre zuul.rb, the following code is run:

Class.class_eval do include Zuul::ValidRoles::ClassMethods end

Why is it adding this behaviour to Class, instead of ActiveRecord::Base? The code to do the latter is even commented out above!

By doing this, now every object now gains a valid_roles method, but this method calls ActiveRecord methods internally, so it’s not actually possible to call it on a ‘normal’ Ruby object without matching ActiveRecord’s API:

module Zuul module ValidRoles # ... module ClassMethods def valid_roles(*roles) attr_protected :role write_inheritable_attribute(:roles, roles) include InstanceMethods end end # ... end end

In my opinion, this is pretty rude. If you were to ask any instance if it responds to valid_roles now, they will always say ‘yes!’ enthusiastically, leaving your code completely obliviously to the fact that calling that method will blow up completely.

Unless there’s a great reason to add something to every object in a running process, we should try to restrict the scope of our behavioural changes as much as possible. In the case of Zuul, valid_roles only belongs on ActiveRecord user classes.