Conditionals (ifs, switches) are a plague in many code bases and although there are campaigns to prevent their spread many developers are unclear on how to control these pests. Conditionals are one of the most prevalent, yet tolerated, sources of repetition and bugs. Why? Well let's start with one simple yet common example:
if(thing.is_valid) do
other_thing.do_something_with(thing)
end
This sort of code spreads like a disease because every time the client calls do_something it must ensure it checks for validity. If we look closer at the object in use we find that calling do_something in an invalid state not only requires the same conditional but also throws an error:
def do_something_with(thing)
raise "Thing must be valid" if not thing.is_valid
// do lots of wonderful things with a valid thing
end
The repetition is not only in the client code but in tests:
def calls_other_thing_when_thing_is_valid
mock_thing.expects(:is_valid).returns(true)
mock_other_thing.expects(:do_something).with(mock_thing)
...
end
def does_nothing_when_thing_is_invalid
mock_thing.expects(:is_valid).returns(false)
...
end
The simplest solution to remove the glut of these conditions (in terms of lines of codes, not calls) is to push it down off the client and to the object who knows his state. If we go back to the tests and use BDD we find that we would rather define the interactions as such:
def tells_thing_to_do_something_when_valid
mock_thing.expects(:when_valid).with(block)
end
Thing now takes a block (in ruby), delegate (in C#) or anonymous class (in Java):
thing.when_valid({other_thing.do_something_with(thing)})
def when_valid(&block)
&block.call if valid
endNow the conditional is controlled by the object responsible for its state: we've removed an Ask and replaced it with a Tell. This is a vast improvement, all the repetition in the clients has gone and the chances of bugs due to accidental calls when in an invalid state are removed.
This presumes that Thing doesn't really do much but chances are it's got lots of behaviour and the chances are that behaviour changes based on being valid or not. Of course all the methods could delegate to its own when_valid method but I bet you also need a when_not_valid and oh boy that code's getting difficult to read. Also, the repetition, in terms of lines of code may be lower but how many times are conditionals executed during run time?
class Thing
def when_valid(&block)
&block.call if valid
end
def other_method
if valid do
// do some stuff
else
// do some different stuff
end
end
end
The conditional can still be pushed further down right to its origin: when Thing changes between a valid and invalid state. This can be achieved by using the Replace Conditional With Polymorphism refactoring to employ the state pattern:
NOTE: I want to keep this example clear for developers of other languages though there are cleverer ways of achieving this in Ruby
class Thing
def validate
valid = // check if valid
@state_behaviour = valid ? Valid.new : Invalid.new
end
def when_valid(&block)
@state_behaviour.when_valid(&block)
end
def other_method
@state_behaviour.other_method
end
class Valid
def when_valid(&block)
&block.call
end
def other_method
// do some stuff
end
end
class Invalid
def when_valid(&block)
// ignore
end
def other_method
// do some different stuff
end
end
end
Now there is only one point where the conditional is called and all of the behaviour for each state (valid or invalid) is in one easy to read place. Of course, this is just a simple example so just imagine what it would do for more complex solutions (and don't forget: the same can be applied to switches).
