Trim Your 'if' Trees

written by ben on September 21st, 2007 @ 12:46 PM

Here’s a helpful little hack for keeping those if trees under control. Ever written code like this:

def parse_line
  if line_is_parsable? 
    if line_is_nested? 
      if line_is_attribute?
        parse_attribute
      else
        # do parsing
      end
    else
      @line_stack.pop
      return
    end
  else
    skip_line
  end
end

What started out as a single if statement has grown into an unmaintainable mess of if*’s *elsif’s, and unless’. Well, thats fine and all, until you have to make changes. What if you need to check for attributes before nested? Or insert a condition check between parsable and nested? Every change requires stepping through the code to figure out whats going on. It’s a drain on productivity, but there is a cure.

First, add the following snippet to your project. (I like to put it in a file called ‘util.rb’ that gets loaded first.)

# Stolen without regard from http://svn.rubyonrails.org/rails/trunk/activesupport/lib/active_support/core_ext/module/aliasing.rb
# Inspired by http://errtheblog.com/post/1109
class Module

  def alias_method_chain(target, feature)
    # Strip out punctuation on predicates or bang methods since
    # e.g. target?_without_feature is not a valid method name.
    aliased_target, punctuation = target.to_s.sub(/([?!=])$/, ''), $1
    yield(aliased_target, punctuation) if block_given?

    with_method, without_method = "#{aliased_target}_with_#{feature}#{punctuation}", "#{aliased_target}_without_#{feature}#{punctuation}"

    alias_method without_method, target
    alias_method target, with_method

    case
      when public_method_defined?(without_method)
        public target
      when protected_method_defined?(without_method)
        protected target
      when private_method_defined?(without_method)
        private target
    end
  end
end

That little bit of magic allows you to break up your if statements into multiple methods. Not so special by itself, until you start chaining the methods:

def parse_line
  #do parsing
end

def parse_line_with_attribute_check
  if line_is_attribute?
    parse_attribute
  else
    parse_line_without_attribute_check
  end
end

alias_method_chain :parse_line, :attribute_check

def parse_line_with_nested_check
  if line_is_nested?
    parse_line_without_nested_check
  else
    @line_stack.pop
  end
end

alias_method_chain :parse_line, :nested_check

def parse_line_with_parsable_check
  if line_is_parsable?
    parse_line_without_parsable_check
  else
    skip_line
  end
end

alias_method_chain :parse_line, :parsable_check

So now the behavior is determined by the code in the methods and the order of methods. This makes changes to the condition checks as easy as inserting, deleting, or reordering your methods. So if I wanted to check attributes before nested, I simply move the attributes check after the nested check:

def parse_line
  #do parsing
end

def parse_line_with_nested_check
  if line_is_nested?
    parse_line_without_nested_check
  else
    @line_stack.pop
  end
end

alias_method_chain :parse_line, :nested_check

def parse_line_with_attribute_check
  if line_is_attribute?
    parse_attribute
  else
    parse_line_without_attribute_check
  end
end

alias_method_chain :parse_line, :attribute_check

def parse_line_with_parsable_check
  if line_is_parsable?
    parse_line_without_parsable_check
  else
    skip_line
  end
end

alias_method_chain :parse_line, :parsable_check

This leaves your code damp, but much easier to extend and maintain. It works great when BDD‘ing.

Comments are closed