Expanding on the Law of Demeter

Recently, my friend Avdi Grim posted a great explanation of the Law of Demeter (LoD). It’s been tweeted and posted all over the web, so hopefully you saw it and read it. I’m totally in agreement with his conclusions in that post, and LoD is a code smell I see in every MVC project I have ever worked in.

I’m going to pick on Avdi’s post just a little, but this is more-so criticism of every LoD explanation I’ve ever read. I think all of these explanations of LoD are actually quite contrite and not addressing the real problem. Avdi (and others) seem to focus on “Structrual Coupling” or exposing the “internal” (actually external*) structure of your objects to other objects that don’t need to know it.

An Example:

def user_info(user) "Name: #{user.name}. Boss: #{user.department.try(:head).try(:name)}" end

Avdi does a beautiful job illustrating why this code is a problem but I want to go a little deeper.

The more insidious LoD violation see in codebases is what I’d call “behavioral” LoD violations, or, when the behavior of a class relies on the behavior of objects that are distant in the object graph.

My example needs a little setup. Lets say you have Vehicles which has_many Platforms which has_many Models which has_many Versions (and Platforms has many Version through models). Pretty simple 4 level deep object graph that a car manufacturer might have. A very common set of objects you might want is to know which Platforms are actually in production. Some our outdated, some are future, but you want all the platforms that are currently in production.

Now, I know how I’d do this, but I commonly see code like this:

class Vehicles
  def current_platforms(options = {})
    platforms = platforms.find(:all,
                        :conditions => 
                          ["platforms.on_date >= ? AND platforms.status IN ('open', 'cancelled') AND platforms.cached_inventory_status != ?",
                            Date.today.to_s(:db), 
                            Platform.cached_inventory_status(:no_inventory)],
                        :order => 'on_date asc, time_note asc',
                        :include => {:models => :versions})
  
    if options[:skip_some_special_case]
      platforms.select { |platform| platform.versions.any? { |offer| !version.special_case} }
    elsif options[:skip_some_other_case]
      platforms.select { |platform| platform.versions.any?(&:other_case?) }
    else
      platforms
    end
  end
end

This code is a great example of my problem with how most folks end up simplifying LoD violations. No where are we chaining methods “too deep” or as it gets simplified to are we “using too many periods”. We’ve only used 1 period (because of a has_many :through =>). However, in our if statements, we are tightly coupling our code to the behavior of a class that is 3 siblings away from the one we’re working in. Nasty. Never should a Platform do operations on, or even know about, a Version.

So why is my example actually worse than something.try(:something_else).try(:a_third)? Well, lets get back to that * I left on “(actually external [structure])”.

Most people seem to classify the active_record generated attribute accessors of Rails model classes as “internal structure” (Avdi doesn’t, I should state for correctness). These are most certainly external structure. You can override them and return whatever you’d like and attributes[:foo] is actually the “internal” accessor for all active_record classes. External structure, in every project I’ve ever worked in, has been the most consistent, least volatile part of the codebases. External structure almost *never* changes, and when it does, it’s typically *very* easy to change (it’s not even a refactoring, it’s just changing the messages exchanged between classes).

My example relies on behavior of a class 3 classes away. If there’s anything I’ve learned, it’s that behavior is always more volatile than structure, and as Avdi points out, LoD really starts hurting as code churns and whatever your LoD relies on changes and becomes more and more complex.

So the big question is, how do we fix this?

It’s actually a theoretically simple fix, and similar to the fixes in Avdi’s post: delegate the behavior down the object graph!

class Vehicle
  def current_platforms(options={})
    if options[:skip_some_special_case]
      platforms.reject(&:special_case?)
    elsif options[:skip_some_other_case]
      platforms.select(&:other_case?)
    else
      platforms.select(&:regular_case)
    end
  end
end

class Platform
  def regular_case
    ...
  end
  def special_case?
    models.special_case?
  end
  def other_case?
    models.other_case?
  end
end

class Model
  def special_case?
    versions.special_case?
  end
  def other_case?
    versions.other_case?
  end
end

class Version
  def special_case?
    ...
  end
  def other_case?
    ...
  end
end

Now, Platform knows nothing about versions, versions can easily change what defines the “cases” without any changes to Platform and ironically, when I made this refactoring to the original source I got this derived example from, it’s actually *more* efficient. Removing the explicit find that started the original method means that if we’ve already loaded this object graph, even outside of this method, we can keep reusing it without going to the database.

I hope this helps, and for the love of Demeter, can we stop describing it as “using too many periods”?

Who should write tests?

The question seems pretty obvious, “Who should be writing the acceptance tests for your software?” I think most people will first answer with “Programmers!”. There might be a few “testers”‘ maybe even a “the customer and programer pair”. These are all fine answers. Unfortunately, I’m starting to think they are wrong.

You are doing BDD right? So you already care about the “b” word: behavior. You care enough to differentiate yourself from the lowly TDD-ers. I’ve got news for you, there is a whole group of folks that care even more about behavior than you do. These people are called Interaction Designers and you probably don’t have one on your team. Don’t get me wrong, everyone that has ever written software has done interaction design, you probably weren’t conscious of it, or if you were, you probably did it poorly (unless you’re an interaction designer, and then you probably don’t read my blog).

Just who is doing IDx in your organization is easy to figure out. When you write a spec, you know, those “it should. . . “, “When” and “Then” statements, who fills in the blank? I’d guess it’s usually the developer (that’s been the case every where I have worked). Sometimes you might ask the customer, product designer, product owner, project manager, designer, or someone else on your team. However, this is where I think we’re doing it wrong. I’m starting to get the feeling that the person that should be filling in these blanks should be the interaction designer (ID). IDs are the people that know your users, they know what your users want, what they need, and what their end goal is. They know where your users sit when they use your app. They know what time of day they use it, they know if they are bored or tired or in a hurry. And most of all, they know what “it should” do and they are right. Do you know this stuff? Do you care? I’ve worked with programmers that were offended to even be in meetings where we talked about personas and end users!

I was recently involved in a little experiment we called “Agile Up To Here” which was (among other things) a 1 week agile bootcamp for Alan Cooper. You might recognize that name if you have ever heard of interaction design because Alan invented the term. He’s written a couple books on the subject, The Inmates Are Running the Asylum and About Face 3: The Essentials of Interaction Design. Anyway, after looking back at the week, I was trying to figure out what we could have done better. We were really agile. We could deploy from day 1, we had CI, we tested the shit out of everything, we paired all the time, we had the customer (Alan) in the room and on the team, it was sweet. But there was still something wrong.

Duh. We had an interaction designer in the room but he wasn’t thinking about IDx, he was learning agile. So who decided what went after “it should” in our app? Programmers, the worse-case choice. And what happened? On Friday, our last day, one of Alan’s IDs paired with Elizabeth Hendrickson and they finally started doing some real interaction design. But it was already too late, the damage was done. We had Interaction Design Debt™.

But why do you need IDx? The job of an ID is to figure out what a user wants to accomplish and the to ensure the application does precisely that. Anything your app does that doesn’t go to this need is waste, and any compromises are mistakes. Programmers, and I say this as someone who has almost solely as a programmer for the last 3-4 years, should be focused solely on figuring out the simplest code that could possibly work and implementing just that. The person that figures out “what could possibly work”, however, should be the ID. This must be a tight loop, and it must take place day in and day out.

Programmers are typically terrible at weighing the needs of the user against all the other things they have to keep in mind. If you don’t believe me, you’re either delusional or a programmer. Which is precisely the problem. This is something most people don’t know they don’t know. Or they don’t know they are actually pretty bad at it. Visual designers can also be pretty bad at IDx. If your designer just hands over Photoshop files with no instructions of how things should behave, thats probably because they are a bad interaction designer.

“Business people” are also usually bad at it, but for almost the opposite reason. Typically I’ve noticed that business people (which usually includes marketing folks) fall into 2 categories. Either they are too solution focused “let’s do it like so and so” without thinking if that’s actually the way it should be done for their users, or they don’t actually care about the implementation. The first group are doing IDx poorly and the second group doesn’t want to write specs. They don’t want to work at the level of thinking required for IDx and programming. They have a business goal. Let’s say it’s that they “want users to be able to sign up for a newsletter”. That’s the level they want to work at. The way the “user does the sign up”, “What happens when they click the “sign up” button?”, “What is written on the button?”, “Does the user have to confirm their sign up?”, and all the 100’s of questions that go along with this feature isn’t the level that most business people want to be involved. And when that is the case, why would  you want them to be involved in writing ATDD tests? If the answer is “because they are the ones that can answer that question” then you’re falling into the same trap that me and a lot of programmers have fallen into, because that just isn’t true. You’re actually looking for an interaction designer, not a business person.

Now that I’ve, I hope, changed your mind about just who should be writing these “tests”, let me get into the part that I’m not quite so confident about. Maybe what I’m talking about shouldn’t actually be expressed as “acceptance tests”. I mean, what happens and what a users sees when they sign up for a newsletter may not really be “business rules” or “business logic”, but isn’t it just as or more important? It’s important enough to me that I’d like to test it and make sure that what I think my code is doing is actually doing what I think it should. In fact, in most of my experience, these types of things are usually much more important and a much larger part of the application than the “business logic”. If you look at something like Facebook, where is there any business logic in that application (is there even any business, I don’t know)? I guess there could be some  in the privacy/permissions area. However, wouldn’t you want to test the multitude of things that happens when someone “likes” something? An interaction designer should be specifying what happens and a developer needs a way to be sure that their code does just that. What do we call those tests? How should we be doing that testing?