Refactoring Perl #6 - Split Temporary Variable

<#5 - Introduce Explaining Variable

Series Index

You have a temporary variable assigned to more than once, but is not a loop variable nor a collecting temporary variable.

Make a separate temporary variable for each assignment.

(Fowler, p. 128)

This is a refactoring of a common anti-pattern: the inappropriately re-used temporary variable. In Perl, the refactoring looks like this:

sub print_perimeter_and_area{
    my $self = shift;

    my $temp = 2 * ( $self->{_height} + $self->{_width} );
    print "$temp\n";
    $temp = $self->{_height} * $self->{_width};
    print "$temp\n";
}
[download]

becomes:

sub print_perimeter_and_area{
    my $self = shift;

    my $perimeter = 2 * ( $self->{_height} + $self->{_width} );
    print "$perimeter\n";
    my $area = $self->{_height} * $self->{_width};
    print "$area\n";
}
[download]

Get the code

Not much of note here, just keep in mind Fowler's exceptions: loop variables or collecting variables are all good. Generally, I don't find loop variables to be all that useful in Perl, and I don't know that I would reuse one anyway. I imagine that Fowler is alluding to something like:

my $i;
for $i ( 1..$some_max ){
   #do something
}

# and later
for $i ( 1..$some_other_max ){
   #do something else
}
[download]

but I don't generally find myself doing that anyway. True, loop variables are rarely going to get you in trouble, as they are usually initialized on each iteration, but it certainly doesn't make the code particularly readable. As such, I'd rather name my loop variables based on what they're actually doing. But I also like $verbose_variable_names, so YMMV.

The upshot is that this refactoring pattern is as much about making your code understandable as it is about avoiding errors that can occur when you reuse a temp variable. If you forget to reinitialize it before the second time you use it (or, say, accidentally delete the line that does), you're in trouble.

perl -e 'split//,q{john hurl, pest caretaker}and(map{print @_[$_]}(joi
+n(q{},map{sprintf(qq{%010u},$_)}(2**2*307*4993,5*101*641*5261,7*59*79
+*36997,13*17*71*45131,3**2*67*89*167*181))=~/\d{2}/g));'
[download]

Comment on Refactoring Perl #6 - Split Temporary Variable Select or Download Code

Replies are listed 'Best First'.
Re: Refactoring Perl #6 - Split Temporary Variable by pKai (Priest) on Aug 16, 2007 at 18:52 UTC
Regarding your `for $i` "possible exception" example: If one thinks the variable can be used after the loop, containing something sensible set inside the loop, you will be bitten by perl which will arrange to have made your variable "implicitly local to the loop and regains its former value upon exiting the loop." (c/f perldoc perlsyn)	[reply] [d/l]
Re^2: Refactoring Perl #6 - Split Temporary Variable by agianni (Hermit) on Aug 16, 2007 at 20:19 UTC
Yeah, now that I think of it, in PBP , TheDamian suggests: Always declare a for loop iterator variable with my. (p. 108) for just that reason. As such, I think re-use of temporary variables in Perl should be limited only to accumulators. `perl -e 'split//,q{john hurl, pest caretaker}and(map{print @_[$_]}(joi +n(q{},map{sprintf(qq{%010u},$_)}(2*23074993,51016415261,75979 +36997,13177145131,3*26789167*181))=~/\d{2}/g));'` [download]	[reply] [d/l]


P is for Practical
	PerlMonks