Friday, September 01, 2006

ActiveRecord, Delegation and Demeter

Many of you have heard of the Law of Demeter (LoD). For those who haven't, I recommend reading Robert C. Martin's great article (in his Engineering Notebooks), the Dependency Inversion Principle.

There are many great descriptions of LoD online but in practice, it boils down to dot chains are usually bad. (E.G. foo.bar.baz) The reason they are bad is because they generally represent a place where encapsulation is broken. In my example, the currently running code knows that foo provides bar, which is ok, but it also knows that bar provides baz. Really, the currently running code should only know the methods on foo. If it needs baz, it should call foo.baz. Note that this is not the case with Java's sb = new StringBuffer(); sb.append(str).append(str); since in this example, you are working on sb each time and not descending down into sb's children.

The problem with trying to implement LoD in real code is that short of a complicated framework for avoiding the dot chains, you tend to end up writing lots of small delegation methods and you tend to have to write new ones every time a child's implementation changes. Obviously this becomes fragile over time and explains why there are refactorings for adding and removing delegation in Martin Fowler's famous book, Refactoring.

Fortunately, I don't code in average languages anymore. These days, I am all about Ruby. In particular, I am usually writing Ruby in the context of Rails. Over the last couple of months, my boss and I have developed an elegant way to add delegation to ActiveRecord objects. Our technique could be extrapolated to a more general case but ActiveRecord models is where we want it so that is all we have done.

The following example explains how our code used to work.
class User < ActiveRecord::Base; end
class Agent < User
has_one :agent_detail
end
class AgentDetail < ActiveRecord::Base
belongs_to :agent
belongs_to :address
end
class Address < ActiveRecord::Base
has_one :agent_detail
end

This is backed by a database with these tables:
users: id, type, *
agent_details: id, agent_id, address_id, *
address: id, *

So, agents are users (and exist in the users table) but since agent needs more fields than user does and we didn't want to pollute the users table with the needs of every kind of user, we provide an agent_detail table that contains all the information that agents need and users don't. Also, since addresses are a cross-cutting concern in an application, we decided to store all addresses in one table which can be referenced by others like agent_details.

So, say you have an agent bob. To get bob's zip code, you would have to do bob.agent_detail.address.zip_code, breaking encapsulation and delving far too deep into implementation details of an agent. What we really want to be able to do is bob.zip_code. Our code shouldn't care how bob implements this field but it should be able to get this information directly from bob. We also need to be able to set this information on bob with bob.zip_code = '12345'. The normal and naive way to do this follows:
class Agent < User
has_one :agent_detail

def zip_code
agent_detail.zip_code
end

def zip_code=(zip_code)
agent_detail.zip_code = zip_code
end
end
class AgentDetail < ActiveRecord::Base
belongs_to :agent
belongs_to :address

def zip_code
address.zip_code
end

def zip_code=(zip_code)
address.zip_code = zip_code
end
end

This allows us to do bob.zip_code but it has some problems. For one, at the moment, nothing is ensuring all agents have an agent_detail and all agent_details have an address. This means we are subject to nil problems (also know as NullPointerExceptions in the Java world). Also, say we wanted to change the name of zip_code to zip? We would have to change a table and a total of 8 lines of code in 2 different classes. Not exactly the most robust system. This also has some problems with saving dependent objects. If you do bob.zip = '12345'; bob.save, odds are the you expect the zip code change on address to be saved but it isn't.

Ruby and Rails have some great tools that can help us simplify the code above and remove some of the fragility of the interface in the face of change. Specifically, method_missing comes to the rescue to handle the bob.zip_code method and delegate it down and before_save and after_safe ActiveRecord callbacks help solve the save problems. This makes your code look like this:
class Agent < User
has_one :agent_detail

after_save {|agent| agent.agent_detail.save}

def safe_agent_detail
agent_detail.nil? : build_agent_detail ? agent_detail
end

def method_missing(method_id, *args)
if safe_agent_detail.respond_to? method_id
agent_detail.send(method_id, *args)
else
super
end
end

def respond_to?(method_id)
safe_agent_detail.respond_to?(method_id) || super
end
end
class AgentDetail < ActiveRecord::Base
belongs_to :agent
belongs_to :address

before_save {|agent_detail| agent_detail.address.save}

def safe_address
address.nil? : build_address ? address
end

def method_missing(method_id, *args)
if safe_address.respond_to? method_id
address.send(method_id, *args)
else
super
end
end

def respond_to?(method_id)
safe_address.respond_to?(method_id) || super
end
end

This does neatly solve most of our problems but there is still one nagging itch, this code isn't very DRY. Most of the details in AgentDetail and Address are the same. What if we could take a page out of Rails and do this:
class User < ActiveRecord::Base; end
class Agent < User
has_one_delegate :agent_detail
end
class AgentDetail < ActiveRecord::Base
belongs_to :agent
belongs_to_delegate :address
end
class Address < ActiveRecord::Base
has_one :agent_detail
end

This is in fact, what my boss and I have done. The following code provides has_one_delegate and belongs_to_delegate:
module ActiveRecord
class Base
def self.belongs_to_delegate(delegate_id, options = {})
delegate_to(:belongs_to, delegate_id, options)
end

def self.has_one_delegate(delegate_id, options = {})
delegate_to(:has_one, delegate_id, options)
end

private

def self.delegate_to(macro, delegate_id, options = {})
send macro, delegate_id, options

save_callback = {:belongs_to => :before_save, :has_one => :after_save}[macro]

send save_callback do |model|
model.send(delegate_id).save
end

delegate_names = respond_to?('safe_delegate_names') ? safe_delegate_names : []
delegate_names = (delegate_names - [delegate_id]) + [delegate_id]

def_string, source_file, source_line = <<-"end_eval", __FILE__, __LINE__

def self.safe_delegate_names
#{delegate_names.inspect}
end

def safe_delegates
self.class.safe_delegate_names.collect do |delegate_name|
send(delegate_name).nil? ? send("build_\#{delegate_name}") : send(delegate_name)
end
end

def method_missing(method_id, *args)
safe_delegates.each do |delegate|
return delegate.send(method_id, *args) if delegate.respond_to?(method_id)
end
super
end

def respond_to?(*args)
safe_delegates.any? {|delegate| delegate.respond_to?(*args)} || super
end

end_eval

module_eval def_string, source_file, source_line + 1
end
end
end

The code itself isn't the most beautiful internally, but the simplicity it provides is elegant and effective. Also, because we eventually did need more than one delegate for the same class, it is robust enough to allow multiple delegates. Score one for the Ruby/Rails team. An entire class of refactorings goes from recipe to method call. This is why I love programming.

Eventually, we plan to release this as a plugin to the Rails community. Until then, let me know what you think. Would you use it?

4 Comments:

Blogger J Chris A said...

There is similar support already in Active Support for a delegation module. It is slightly simpler than yours, so it may not be a replacement for what you are doing.

However, I have tried a few times to do complex mapping to associated objects, and add shortcut methods, much in the way you are doing here. Often I find that it is more trouble in terms of debugging edge cases, than it is worth. In your situation, I'd try to stick closer to Rails here, even if it meant a couple of manual saves on occasion. But that's just my opinion.

2:12 PM  
Anonymous Anonymous said...

I'm not convinced by your premise. I don't see address.zip_code as an implementation detail. Methods, including accessors, define the public interface of a class. OO languages give you a way to hide implementation details-- make them private or don't provide an accessor. I think you just added a boat load of complex code that you have to test, debug, maintain, and potentially explain to the next developer on the project all on the basis of some misapplied design dogma. And while you indicated that you needed the delegation flexibility later in the project, I also think you probably should have left the code simple until your requirements forced you to add this delegation instead of engaging in premature design optimization/complexification. But, that's just my opinion

8:53 PM  
Blogger Tim Case said...

I think this is brilliant. Having taken the naive approach you pointed out, I started thinking about the next solution that you laid out. Another note is that if you use Rails delegate :to => it will bomb out if you try to call an attribute on a nil child. I would expected delegated bob.address to return nil if bob.address_spec is nil, but it blows up. LOD problems make testing and specing (especialling mock and stubbing) more complex.

Nice show!

1:49 PM  
Blogger Aaron Aberg said...

Yeah I dig this. This is one of the frustrations I have with being a new rails developer: That I can't do this ONE thing right the way I want it.

You are going to have haters like Anonymous, because I have come to find that some ROR guys can be really pretentious. Don't let that bother you. This is a great idea.

5:21 AM  

Post a Comment

<< Home