Classes are separations of concerns or why too much magic hurts

I am currently working on a project that requires me to scope nearly all database queries by customer_id, because, you probably get the idea, each and every customer has its own set of data. Furthermore, the Customer class does not inherit from ActiveRecord::Base, nor is it an ActiveModel class.

So in order to create the scopes for all the classes, I created the following module, which is included in all the classes needing the scope:

# app/models/shared/customer.rb
module Shared
  module Customer
    extend ActiveSupport::Concern

    included do
      scope :for_customer, ->(customer) do
        where(:customer_id => customer.to_i)
      end
    end
  end
end

# app/models/user.rb
class User < ActiveRecord::Base
  include Shared::Customer
end

This scope helps already, but in the end I want to do the following in my controllers:

@users = customer.users.all
@user = customer.users.find(params[:id])
@user = customer.users.new(params[:user])

So for each model including the shared customer module, I need to create a scope like that:

# app/models/customer.rb
class Customer
  def users
    User.for_customer(id)
  end
end

That seemed like to much work for me. I mean, we are using Ruby, and I do not want to do boring routing stuff. So I created the following magic which frees me of the task to create this association method by hand for nearly each and every model:

# app/models/shared/customer.rb
module Shared
  module Customer
    extend ActiveSupport::Concern

    included do |base|
      scope :for_customer, ->(customer) do
        where(:customer_id => customer.to_i)
      end

      Customer.class_eval <<-EOS
        def #{base.tableize}
          #{base}.for_customer(id)
        end
      EOS
    end
  end
end

No more routine work, wheeeee! One might think. But of course, this code is causing problems. First of all, the load order matters now. If we log into rails console and try calling the users method on a customer object, we will be out of luck. This is because the Customer class by itself does not know that there might be another class putting code into the Customer class at some later point of time. And requiring all the classes by hand would make a mockery of the whole automation idea, plus it does not look very clean. This is not a problem in the production and test environment, because all the models get loaded there. But it will definitely fall on your feet while you are developing stuff, trust me.

The other problem is readability: How the heck is someone reading the Customer class supposed to know where the association comes from? And if she/he finds out after 2 hours, do not be surprised if she/he shouts death threats in your direction. So in the end I abandoned the whole idea, and settled for something else. I just created a new concern for the customer class, taking care of the associations.

# app/models/customer/associations.rb
class Customer
  module Associations
    extend ActiveSupport::Concern

    module ClassMethods
      def associate_with(*item_names)
        item_names.each do |item_name|
          method = -> { item_name.to_s.classify.constantize.for_customer(id) }
          define_method(item_name, method)
        end
      end
      private :associate_with
    end

    included do
      associate_with :users, :pizzas, :schnitzels
    end
  end
end

# app/models/customer.rb
class Customer
  include Associations
end

As you can see in the snippet, I still have to add each class that needs to be associated with Customer to the associate_with macro. But I can live with that. It's the best solution I could find, finally. Adding a word to the macro ain't too hard, and you can easily find the source of the association.

Plus, this code looks very clean to me. Classes should not know too much of each other, they may share data internally, but they ought to have a well defined interface externally. So a class walking over to another class, modifying its interface, that's just not very clean.

Conclusion:

Use metaprogramming to free yourself from routine programming tasks and to create clean and concise code. But don't be too clever, and always think about the readability of your code. Because, remember, kids: Code is read much more often than it is written.

Conclusion 2:

Discussing stuff with other developers (Lee in this case) will make you a better programmer, in the end. Heck, it's the reason why I'm bloggind this!