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.