Cykod

Ugly Abstraction, or "Just one more option parameter"

by Pascal Rettig posted May 07, 2010

One of the mistakes that I was (and probably still am - but being aware of the problem I think has helped) guilty of was the crime of trying to make one piece of code do too many things.

Like a vacuum cleaner with too many useless attachments that don't work correctly and keep getting lost, expanding on one function or piece of code by adding too many different conditional branches leads to a quick and ugly code death. At first glance, adding options might seem like vintage DRY - if you already have code that "almost" performs a certain function, why not add a couple of lines and a parameter or two to make it do a little bit more. In reality though, a hundred and one conditional branches are the quickest way to take an ugly-stick to your code. The next guy forced to look at the spaghetti that you've written will probably just end up quitting so that he doesn't have to maintain what you've written.

The better option, which is still DRY but also allows for separation of concerns is to extract the shared functionality separately and then invoke both your old function and a new function to get job done. To give a simplified example (extracted from some old PHP code I looked at recently), what's better:

function generate_table(&$data,$wrap_in_a_div = false) {
$table_info = ... // Generate your table
if($wrap_in_a_div} {
return "<div>" . $table_info . "</div>";
}
else {
return $table_info;

}

Or:

function generate_table(&$data) { 
$table_info = ... // Generate your table
return $table_info;
}
function generate_table_in_div(&$data) {
return "<div>" . generate_table($data) . "</div>";
}

 

Ignoring the triviality of the example, for my money, the second one is liquid gold compared to the first one. There may be some perfectly valid need to have a table in a <div> tag sometimes and not sometimes, but the table generating code shouldn't worry about it because it's not it's job. If it turns out you need a whole bunch of junk occassionally wrapped in a <div>, you can down the road extract a generic wrap_in_div method and refactor generate_table_in_div to us that method.