Trim Your 'if' Trees
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.