Polymorphism instead of conditionals

Ovid on 2006-01-17T23:24:06

Some days I just get mental blocks. I had some ugly code which was trying to test which server I wanted to start and the code was filled with if/elsif/else blocks depending upon the server I wanted to launch. There's something about trying to work with servers and launch new processes which causes me to forget about robust language design. Then my boss asked my why I was doing it the way I was doing it and it took me a bit to realize I was being foolish. Now, my large, ungainly code has been reduced to this (pared down for readability):

my $command = lc shift @ARGV || '';
unless ( $command =~ /^stop|start|restart$/ ) {
    die localize("You must specify stop, start, or restart");
}

my $engine = ENGINE_CLASS;   # loaded elsewhere
eval "use $engine";
if ( my $error = $@ ) {
    die localize("Could not load [_1]: [_2]", $engine, $error);
}

$engine->$command;

Now, each engine class implements its own start, stop, and restart methods and all of the code is much cleaner. It bugs me when I get mental blocks like this.


classic Design Pattern

jmason on 2006-01-18T02:12:29

this, btw, is a classic Design Pattern, and is in the Gang of Four book of the same name. can't remember which one, though. ;)

How are you doing that localization, as a matter of interest?

Re:classic Design Pattern

jmason on 2006-01-18T02:14:37

aha. got it -- it's the Command pattern.

Re:classic Design Pattern

Ovid on 2006-01-18T02:34:32

The localization is done via an in house module which subclasses from Locale::MakeText. As for the "pattern", I was thinking more of the classic refactoring pattern of exchanging conditionals for polymorphism.

Regexp sillyness

bart on 2006-01-20T10:25:55

unless ( $command =~ /^stop|start|restart$/ ) {

You forgot to group the alternatives. Now you're saying "If this command starts with "stop", contains "start" or ends with "restart" (which also contains "start", BTW), then everything is alright.

To fix it:
unless ( $command =~ /^(?:stop|start|restart)$/ ) {

Re:Regexp sillyness

Ovid on 2006-01-20T18:08:34

Oh, sheesh. You're right. I feel silly and thanks for the catch :)