Working with legacy code

Delivering Christmas gifts is a hard job. Every culture and every country has it's own different traditions. Each country has it's own different gift bringer - an old man (Santa Claus, Father Christmas, Grandfather Frost, ...), a child (Baby Jesus, Christ Child, Child God, ...) or other (The Three Kings, Yule Goat, ...) And each country has different days, from Christmas Eve, or January 7th following the Gregorian calendar, or even January 2nd in Japan.

All these entities worked together in the Society of Annual Noel Transfer Association (S.A.N.T.A) to coordinate their deliveries. And it was someone's job to keep track of what everyone had to do.

To help with this problem they'd started with a deliver_gift subroutine. Well intention, well named, but over time as more members had joined S.A.N.T.A. it'd grown little bit out of control:

sub deliver_gift {
   my ($gift, $child, $location, $delivery_day, $old_man_bringer, $delivery_method, $child_bringer, $other_bringer) = @_;

   # few hundreds of lines of code untouched this millennium follows
   ...
}

Nightmare Before Christmas

The Nightmare begins for our maintainer - this function needs yet more changes. He needs to add a delivery_time argument to account for the time of day when presents are delivered (which again, apparently, varies between country).

Now:

• only first three arguments are mandatory
• function is called from many places
• optional arguments are often omitted

A Bad Solution

The hacky solution that maintainer can implement is to put a new argument after last mandatory field. Pity our poor maintainer who chooses this solution and failed to update every last place that the function is called in the production code and accidentally calls the function with an argument in the wrong place!

A Slightly Better Solution

Our poor maintainer wants to spend Christmas at home with his family, not on call for bad code changes! At this point his slightly better solution is simple: Quit his job and enjoy peace and harmony on earth!

The Better Solution

It was time for our maintainer to bite the bullet and change the function's calling style from positional to named arguments so it can be called like so:

deliver_gift (
    gift          => $gift,
    child         => $child,
    location      => $location,
    ...,
    # our new functionality
    delivery_time => 'after-dinner',
 );

But how to do that without breaking all the existing code?

Plan A: deliver_gift_with_named_arguments

Our maintainer's first plan was a multi-step process:

• implementation: introduce a new function
• implementation: make the old function wrapper over the new one
• implementation: replace the old function's calls with new one
• cleanup: rename the new function into the old name, create a wrapper
• cleanup: replace the new function name with the old name
• cleanup: finally get rid of the wrapper

Oh My! So many steps!

In these days of zero downtime deployment via microservices a lot of synchronization points are required for such a complicated plan, multiplied by number of modules that uses this function.

Could we get all this done by Christmas? Unlikely! Our maintainer needed a better plan!

Plan B: Make function to accept both positional and named arguments

There is a handy module on CPAN called Sub::Params that handles calling style duality.

use Sub::Params qw[ named_or_positional_arguments ];
sub delivery_gift {
   my ($self, @args) = @_;

   my %args = named_or_positional_arguments (
      args => \@args,
      names => [
          'gift',
          'child',
          'location',
          'delivery_day',
          'old_man_bringer',
          'delivery_method',
          'child_bringer',
          'other_bringer',
      ],
   );
}

With these changes the function can be called in either style:

delivery_gift (
   $gift,
   $child,
   ...
);

delivery_gift (
   gift  => $gift,
   child => $child,
   ...
);

Reducing the number of necessary changes into merely:

• implementation: introduce duality
• implementation: change positional style to named style
• cleanup: remove duality

Merry Christmas and Happy Refactoring :-)

Thanks

GoodData for contributing to open source.