Jake Douglas

writing code that does stuff

Archive for the ‘Code design’ Category

Every day use of the decorator pattern

with 4 comments

At some point I found that my tests were too complicated and hard to maintain. I saw people talking about design patterns, but assumed that they were an Enterprise Thing that I should stay away from. It turns out that design patterns are useful even in vanilla Rails programming and have significantly improved the quality of my code.

James Golick has a great gentle introduction to some of these ideas and why they lead to more easily tested and maintainable code. I thought it was worth sharing some examples that might help the principles click with more people. This post will discuss the decorator, and assumes that you are familiar with some basic concepts like dependency injection.

Lets take an example from our social network application. Users have nicknames and we’re writing some code that lets a user change their nickname. Obviously we need to update the user’s database record with the new nickname, but there are a few other things:

  1. We need to keep a history of which nicknames a person has used. To do this we’ll create a NicknameHistory record when they successfully change their nickname.
  2. A user is only allowed to change their nickname every 28 days. You can imagine that the historical records we’re keeping would allow us to determine whether a user is eligible, but for this example you should take it for granted that the NicknameChangePolicy implements this logic correctly.
  3. We want to send an email confirmation to the user after they successfully update it.

We’ll start with one new class that is going to perform these duties. It will also return a boolean to indicate whether or not the update succeeded.

class NicknameUpdateService
  def initialize(nickname_change_policy = NicknameChangePolicy.new,
                 nickname_history_klass = NicknameHistory,
                 mailer                 = SomeMailer)
    @nickname_change_policy = nickname_change_policy
    @nickname_history_klass = nickname_history_klass
    @mailer                 = mailer
  end

  def update(user, nickname)
    old_nickname = user.nickname

    if @nickname_change_policy.can_proceed?(user)
      user.update_attributes(:nickname => nickname).tap do |was_successful|
        if was_successful
          @nickname_history_klass.create!(:from => old_nickname,
                                          :to   => nickname,
                                          :user => user)

          @mailer.deliver_nickname_change_notification(user)
        end
      end
    else
      false
    end
  end
end

This code fulfills all of the requirements, but theres a lot going on and the behaviors are tied together instead of being re-usable individually. Also, look at the RSpec specification for such a class:

describe NicknameUpdateService do
  context "given the user is eligible to update their nickname" do
    context "and the update succeeds" do
      it "creates a NicknameHistory record"
      it "sends a notification email"
      it "returns true"
    end

    context "and the update fails" do
      it "doesn't create a NicknameHistory record"
      it "doesn't send a notification email"
      it "returns false"
    end
  end

  context "given the user is not eligible to update their nickname" do
    it "doesn't try to update the user record"
    it "doesn't create a NicknameHistory record"
    it "doesn't send a notification email"
    it "returns false"
  end
end

We have to test every conditional branch in the method, so in this case we already have 3 scenarios. Imagine that we wanted to add another condition, that the email confirmation should only be sent if it’s also a Tuesday. The complexity of the tests quickly balloons and they become hard to maintain and extend.

Considering the Single Responsibility Principle, each behavior should probably be factored out into its own class. These classes can then be chained together, each one of them “decorating” the behavior of the class they wrap with its own. We can start with the most basic class that simply updates the user record:

class Nickname::UpdateService
  def update(user, nickname)
    user.update_attributes(:nickname => nickname)
  end
end

Next, we’ll decorate the base class with the creation of a historical record when the update succeeds.

class Nickname::UpdateServiceWithHistory
  def initialize(update_service         = UpdateService.new,
                 nickname_history_klass = NicknameHistory)
    @update_service         = update_service
    @nickname_history_klass = nickname_history_klass
  end

  def update(user, nickname)
    old_nickname = user.nickname

    @update_service.update(user, nickname).tap do |was_successful|
      if was_successful
        @nickname_history_klass.create!(:from => old_nickname,
                                        :to   => nickname,
                                        :user => user)
      end
    end
  end
end

Then we have to send the email.

class Nickname::UpdateServiceWithEmailNotification
  def initialize(update_service = UpdateServiceWithHistory.new,
                 mailer         = SomeMailer)
    @update_service = update_service
    @mailer         = mailer
  end

  def update(user, nickname)
    @update_service.update(user, nickname).tap do |was_successful|
      if was_successful
        @mailer.deliver_nickname_change_notification(user)
      end
    end
  end
end

Finally, the outer-most decorator needs to ensure that none of this takes place unless the user is eligible to change their nickname. This is the class that would actually get called from a controller or similar.

class Nickname::UpdateServiceWithEligibilityAwareness
  def initialize(update_service         = UpdateServiceWithEmailNotification.new,
                 nickname_change_policy = NicknameChangePolicy.new)
    @update_service         = update_service
    @nickname_change_policy = nickname_change_policy
  end

  def update(user, nickname)
    if @nickname_change_policy.can_proceed?(user)
      @update_service.update(user, nickname)
    else
      false
    end
  end
end

There are a number of resulting benefits:

  1. Each piece of behavior can have its own unit tests that only have to consider a maximum of two branches.
  2. Each piece of behavior can be re-used independent of the other behaviors.
  3. Adding new behavior does not require changing the existing behaviors, only the chain of classes. This is made particularly easy since the interfaces are all identical, or in the case of Ruby, just the method signature and the meaning of the return value.

There are a few disadvantages as well, that I consider to be negligible in light of the benefits:

  1. We have to write extra boilerplate to define the additional classes and tests.
  2. The entire set of behavior cannot be understood by reading a single method, and instead requires looking at multiple classes and how they decorate one another. I haven’t had a problem with this, but using a factory might help make it more clear which decorators are being used.

A more diligent tester might create a factory class and a corresponding unit test to verify that the decorators are being chained together correctly. However, I haven’t been bitten by not doing this yet, and we rely on integration tests for verifying behavior that we consider to be critical.

Advertisements

Written by jakedouglas

October 16, 2011 at 6:31 pm

Posted in Code design