Perl Best Practices

17.2. Refactoring

Place original code inline. Place duplicated code in a subroutine. Place duplicated subroutines in a module.

The first time you're tempted to copy-paste-and-modify a piece of code:

package Process::Queue; use Carp; { use overload ( # Type coercions don't make sense for process queues... q{""} => sub { croak q{Can't stringify a Process::Queue}; }, q{0+} => sub { croak q{Can't numerify a Process::Queue }; }, q{bool} => sub { croak q{Can't get the boolean value of a Process::Queue }; }, ); } # and later... package Socket; use Carp; { use overload ( # Type coercions don't make sense for sockets... q{""} => sub { croak q{Can't convert a Socket to a string}; }, q{0+} => sub { croak q{Can't convert a Socket to a number}; }, q{bool} => sub { croak q{Can't get the boolean value of a Socket }; }, ); }

. . . don't do it!

Instead, convert the code into a subroutine, parameterize the parts you would have modified, and then replace both the original and duplicated code with calls to that subroutine:

use Carp; sub _Class::cannot {

# What kind of coercion cannot be done? my ($coerce) = @_;

# Build a subroutine with the corresponding error message... return sub { my ($self) = @_; croak sprintf qq{Can't $coerce}, ref $self; }; }

# and later... package Process::Queue; { use overload (

# Type coercions don't make sense for process queues... q{""} => _Class::cannot('stringify a %s'),

q{0+} => _Class::cannot('numerify a %s'),

q{bool} => _Class::cannot('get the boolean value of a %s'), ); }

# and later still... package Socket; { use overload (

# Type coercions don't make sense for sockets... q{""} => _Class::cannot('stringify a %s'),

q{0+} => _Class::cannot('numerify a %s'),

q{bool} => _Class::cannot('get the boolean value of a %s'), ); }

This refactoring might produce slightly more code, but that code will be cleaner, more self-documenting, and easier to maintain. And the next time you need the same functionality, the total amount of code will almost certainly be less than cutting and pasting would have produced.

Note that factoring out the messages still left a chunk of repeated code in each class. In such cases you should re-refactor the code:

use Carp; sub _Class::cannot {

# What kind of coercion cannot be done? my ($coerce) = @_;

# Build a subroutine with the corresponding error message... return sub { my ($self) = @_; croak sprintf qq{Can't $coerce}, ref $self; }; } sub _Class::allows_no_coercions { return ( q{""} => _Class::cannot('stringify a %s'),

q{0+} => _Class::cannot('numerify a %s'),

q{bool} => _Class::cannot('get the boolean value of a %s'), ); }

# and later... package Process::Queue; {

# Type coercions don't make sense for process queues... use overload _Class::allows_no_coercions( ); }

# and later still... package Socket; {

# Type coercions don't make sense for sockets... use overload _Class::allows_no_coercions( ); }

The first time you're tempted to copy and paste a subroutine definition into some other file, program, or system...don't do that either! Instead, place the subroutine in a module and export it:

package Coding::Toolkit::Coercions; use Carp; sub _Class::cannot {

# What kind of coercion cannot be done? my ($coerce) = @_;

# Build a subroutine with the corresponding error message... return sub { my ($self) = @_; croak sprintf qq{Can't $coerce}, ref $self; }; } sub _Class::allows_no_coercions { return ( q{""} => _Class::cannot('stringify a %s'),

q{0+} => _Class::cannot('numerify a %s'),

q{bool} => _Class::cannot('get the boolean value of a %s'), ); } 1;

# Magic true value required at the end of any module

Then import it wherever you need it:

use Coding::Toolkit::Coercions; package Process::Queue; {

# Type coercions don't make sense for process queues... use overload _Class::allows_no_coercions( ); }