Classes are separations of concerns or why too much magic hurts
» Posted on 2011-12-02I 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 don't 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'll 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're 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, don't 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!