Catalyst Model #10: Fixing your legacy code by not fixing it

Published · Friday, 24 July 2009 (Updated · 7 March 2010)

You just inherited—or perhaps woke up sober to discover you wrote—a mound of code spaghetti in perl4/cgi-lib.pl style which uses neither strict nor SQL placeholders nor sanity. It cannot be used with your shiny new MVC because it’s got no separation of concerns and it’s bleeding namespaces and SIG handling like a stuck pig in a wedding dress.

“Trash it! Immediately and forever!” you cry. Nice idea. The cats who built it—it really wasn’t you, was it? Oh, well, karma isn’t always a no op—tied it deeply into other parts of your code and your presentation “layer” and it’s just too big and awful to chuck without spending 6 months to rewrite. Plus you have no tests so you don’t even know if your code is supposed to stay “broken” in places because it’s what customers expect or has necessary, undocumented side-effects.

So, what do you do? To your manager it’s obvious that you can’t just start over. To you it’s obvious that moving forward by adding to the pile could be construed as a terrorist act. What can you do?

Do what any good sanitization operation does: put the crap in a bag.

The legacy codeproblem

Anyone who has been around awhile will find this code hauntingly familiar. I’ve seen its ilk at almost every contracting job I’ve had. This stems from false laziness and over-hurried developers. Perl can solve pretty much any problem and it can do it, generally, dramatically faster than other programming languages. This leads to false economy.

Perl hackers, and I’ve been one of the offenders, too often rush through to the first solution that presents itself in too small a scope with no eye toward the future of the code. We get a prototype running so fast it makes the project manager’s head spin. Then we have a pile of code which over time costs the maintenance hackers 10-fold, at least, what it would have cost to slow down a little and do it right the first time.

The moral? Over-estimate on your projects. Use the padding to write tests and maybe documentation first. Please see #9 in The 10 biggest mistakes Perl hackers make. Tests are easy to write in Perl. Sometimes trivially so. Writing tests first is the single best method for shaking out bad designs.

Now for our sample legacy code. I only wish I could tell you it was difficult to write. :)

In the spirit of fighting fire with fire, we’re going to fight voodoo with voodoo. Most of that we can fix with some plain, if somewhat abstruse, Perl. Something that normally requires a lot of verbosity to fix is the dumber routine printing and if we had a real package we might have a bunch of that shite. So we want a general solution that’s already debugged and set-up for us. IO::CaptureOutput is a great fit for this task.

Install the dependency

cpan IO::CaptureOutput

Tactics to use the code without doing harm

We’ll put our legacy code into a model within a “secret” namespace, and prevent it from messing with global space or bombing our entire application. What we need to do to fix the evil points above–

Nothing, c’ést la vie. It works as is, we can’t fix this without risking breaking the code’s current behavior in other places.

We could do two things–

Don’t let it pollute %ENV with evil.

Don’t let it have the shell at all so cat won’t execute.

–we’ll do both though you might have to pick an approach or lose valid functionality. Either would fix this particular problem.

We’ll neuter exit for that scope.

We’ll cache the global signals and put them back after loading our legacy code.

IO::CaptureOutput, as foretold.

Again, see #1, Perl doesn’t care so we don’t.

Perl only warns, we don’t have warnings on, so we don’t have to fix anything.

See #1.

Then we will have a situation where you have access to your legacy code when needed without letting it contaminate the new shiny stuff with flecks of feces.

Da model

./script/myapp_create.pl model Legacy

emacs lib/MyApp/Model/Legacy.pm

package__iHAZaSAD;# Namespace for isolation.nowarnings;uselib"./legacy/lib";my$legacy_lib="legacy-lib.pl";# Save some important things.
my$_exit=\*CORE::GLOBAL::exit;my%_sig=%SIG;my%_env=%ENV;# Neuter this so the legacy code can't use it.
*CORE::GLOBAL::exit=sub{0};@ENV{qw( PATH HOME SHELL USER )}=()x4;# Do the filthy, messy deed.
require$legacy_lib;# Put things back where they belong.
*CORE::GLOBAL::exit=$_exit;%SIG=%_sig;%ENV=%_env;# Back to regular space --------------------------------
packageMyApp::Model::Legacy;usestrict;useparent"Catalyst::Model";useCarp();useIO::CaptureOutputqw(capture);our$AUTOLOAD;subAUTOLOAD{my$called_sub=$AUTOLOAD;(my$sub_name=$called_sub)=~s/\A.+:://;my$legacy_function=\&{"__iHAZaSAD::".$sub_name};{nostrict"refs";*$called_sub=sub{ # We don't need or want "$self," we're just wrapping
# legacy-lib.pl's exported functions from an OO call.
my($self,@args)=@_;defined&$legacy_functionorCarp::croak("$legacy_lib has no $sub_name function");my(@return,$return,$out,$err);if(notdefinedwantarray){capture{$legacy_function->(@args)}\$out,\$err;Carp::carp($err)if$err;return;}elsif(wantarray){@return=capture{$legacy_function->(@args)}\$out,\$err;Carp::carp($err)if$err;return@return?@return:$out;}else{$return=capture{$legacy_function->(@args)}\$out,\$err;Carp::carp($err)if$err;return$out?$out:$return;}};}goto&{$called_sub};}subDESTROY{1}subget_variable{my($self,$variable)=@_;nostrict;${"__iHAZaSAD::$variable"};}subget_hash_value{my($self,$variable)=@_; # Don't autovivify.
exists$__iHAZaSAD::LEGACY_HASH{$variable}?$__iHAZaSAD::LEGACY_HASH{$variable}:undef;}1;

Take a minute to explicitly tell yourself that this kind of approach is a shim, not code you want to live with long. I do advocate this sort of trickery in production environments if it helps move the code base forward. If it ends up being another level of complexity without tests, without documents, without taking any of the lessons from the first pass then don’t do it. Stick with the old code base till your manger drinks the Ruby or C# or Python or .NET or Java Kool-Aid®.

If you want Perl to be taken seriously in your shop, you—the developer—need to be serious about writing it first.

Now one controller to rule them all… well, to use new and old stuff together

Run it and see if it’s salvation or doom

./script/myapp_server.pl -d -r -p 3000

That’s it, kids! We’re done with the ten. Woo-woo! Come back in a few for the wrap-up and for the source code for the whole project + a nice index, suggested, exercises, and ideas for extensions. Don’t hesitate to let me know if you see problems, improvements, or clarifications.