A good function gone bad

One of the nice features about where I work is that I have switched the division from a really crap old proprietary, windows based billing system, to a “Perl based, GPL billing system”:http://www.sisd.com/freeside using Postgres (optionally MySQL) as the back end.

One of the main tasks I have at work is “Enterprisifying” this system. Making sure it can handle enterprise level features that my company needs.

Right now I’m working heavily on caching mechanisms and middleware storage, turning it into more of an N-tier architecture, and I’m looking at some existing code that starts with…

bc.
$statement .= ‘ WHERE ‘ . join(‘ AND ‘,
( map {

p. A really simple SQL building block (that’s another issue altogether) but the problem is what comes next. That join of course means there is a list next, but the map has a ‘(‘ before it. That’s right, there is more than one (two in fact) which take up the next *2 pages*. ARRRGHH! What were you thinking people? Yes it’s possible, might have even saved 2 variables, but it sure didn’t help readability. This should most positively have been broken out into multiple statements, over even better, re-factored into some subroutine call or similar visual optimization. Two *pages* of code written in what is a single line tool. It will look so much nicer when it’s more like:

bc.
$statement .= ‘ WHERE ‘ . join(‘ AND ‘, define_columns( @data ) );

p.
Oh well, I have the source, I have cvs, I can correct this blasphemy.