Beefy Boxes and Bandwidth Generously Provided by pair Networks
go ahead... be a heretic

Refactoring Perl #7 - Remove Assignments to Parameters

by agianni (Hermit)
on Aug 23, 2007 at 04:01 UTC ( #634565=perlmeditation: print w/replies, xml ) Need Help??

The code assigns to a parameter.

Use a temporary variable instead.

(Fowler, p. 131)

Initially this may strike you as a simple refactoring pattern and the mechanics of it are. Here's how it looks in Perl:

sub discount{ my $arg_ref = shift; $arg_ref->{input_val} -= $arg_ref->{input_val} > 50 ? 2 : 0; $arg_ref->{input_val} -= $arg_ref->{quantity} > 100 ? 1 : 0; $arg_ref->{input_val} -= $arg_ref->{year_to_date} > 10000 ? 4 : 0; return $arg_ref->{input_val}; }


sub discount{ my $arg_ref = shift; my $input_val = $arg_ref->{input_val}; $input_val -= $arg_ref->{input_val} > 50 ? 2 : 0; $input_val -= $arg_ref->{quantity} > 100 ? 1 : 0; $input_val -= $arg_ref->{year_to_date} > 10000 ? 4 : 0; return $input_val; }

Get the code

Fowler's primary reason for promoting this pattern is to address the "lack of clarity and to [sic] confusion between pass by value and pass by reference". (p. 131)

Note: my original writeup contained a number of factual errors, which I believe I have corrected.

This is important in Java for reasons that Fowler explains, but I don't care to pretend I know or understand Java enough to elaborate. In Perl everything is passed by reference. Updating a parameter passed as a reference -- i.e. modifying $_[$x] or a copy of a reference contained therein -- could have unexpected consequences and is best avoided. Thanks to ikegami for pointing out that Perl is always pass by reference, contrary to my original suggestion to the contrary.

The exception would be in the situation where a subroutine would otherwise modify and return a very large data structure or object or where you will call a subroutine many, many times to modify and return any data passed to it. In these situations, it may be appropriate to pass in a reference, modify it within the subroutine and not return it. This is not a pre-optimization that I would recommend, as I think it makes code less maintainable, but it is an optimization to be considered if you find your code to be slow. I should point out that this is much less likely to be a problem in Perl than in other programming languages, but there are situations where it is an appropriate technique.

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));'

Replies are listed 'Best First'.
Re: Refactoring Perl #7 - Remove Assignments to Parameters
by ikegami (Pope) on Aug 23, 2007 at 14:59 UTC

    In Perl, of course, you generally have the option of using pass by value or pass by reference.

    Everything is passed by reference in Perl.

    sub foo { $_[0] = 'bar'; } foo($a); print($a); # bar

    Don't confuse "passing by reference" with "passing a reference".

      OK, so I am confused, what about this:
      my $var = 'foo'; warn $var; # prints foo by_val( $var ); warn $var; # prints foo by_ref( \$var ); warn $var; # prints bar sub by_ref{ my $var_ref = shift; $$var_ref = 'bar'; } sub by_val{ my $var = shift; $var = 'bar'; }

      My understanding was that this is PbV and PbR respectively. Your post suggests that Perl is always PbR, so does that mean that my pass by value example is just allowing me to fake PbV? Or is the difference more of a technical, low level detail?

      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));'

        by_val does not modify its parameter ($_[0]), it modifies a copy of its parameter ($var). Without realizing it, you've used the very refactoring tool you are trying to present to us (my $var = shift;).

        Using a temporary variable instead of assigning to (or even reading from) a parameter is standard practice in Perl!

        Update: Example:

        sub inc_original { $_[0]++; return $_[0]; } sub inc_refactored { my ($var) = @_; $var++; return $var; } { my $i = 3; my $j = inc_original($i); print("$i + 1 = $j\n"); # 4 + 1 = 4 } { my $i = 3; my $j = inc_refactored($i); print("$i + 1 = $j\n"); # 3 + 1 = 4 }
Re: Refactoring Perl #7 - Remove Assignments to Parameters
by Anonymous Monk on Aug 23, 2007 at 13:50 UTC
    You say "... that objects are generally blessed references (although they don't have to be)".

    What else can they be (other than blessed references)?

      Err... you're right, for some reason I was thinking you could bless a simple scalar, but that's not the case. Not sure what I was thinking.
      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));'
      What else can they be (other than blessed references)?

      With the autobox module, bultin datatypes can be used as first class objects.

Re: Refactoring Perl #7 - Remove Assignments to Parameters
by hossman (Prior) on Aug 23, 2007 at 20:39 UTC

    even after your update, you probably still want to change the main example in your post ... the initial code black already has the param assigned to a temp variable in the my $arg_ref = shift; line and demonstrates the spirit of the refactoring -- it should be your "post refactoring" example. Your "pre refactoring" example should have every line modifying $_[0] directly.

      Actually, I think the example code as it stands is in the spirit if not the word of what the refactoring pattern is meant to avoid. While $arg_ref is a local temp, it's still a reference to the original arguments (a hashref), so updating it would update the arguments pass, which is what this pattern is meant to address. Hence the introduction of the secondary temp. That said, for the sake of completeness, the completely un-refactored code might look like this as you suggest:
      sub discount{ $_[0]->{input_val} -= $_[0]->{input_val} > 50 ? 2 : 0; $_[0]->{input_val} -= $_[0]->{quantity} > 100 ? 1 : 0; $_[0]->{input_val} -= $_[0]->{year_to_date} > 10000 ? 4 : 0; return $_[0]->{input_val}; }
Re: Refactoring Perl #7 - Remove Assignments to Parameters
by Anonymous Monk on Aug 23, 2007 at 13:48 UTC
    Wow!!!, almost everything in your post is wrong!!!
      Would you care to elaborate? My hope is to make this series of nodes a community endeavor not just my own personal project, so if you see something wrong, point it out and I will be happy to correct it.
      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));'

Log In?

What's my password?
Create A New User
Node Status?
node history
Node Type: perlmeditation [id://634565]
Approved by GrandFather
and all is quiet...

How do I use this? | Other CB clients
Other Users?
Others perusing the Monastery: (1)
As of 2017-03-26 03:41 GMT
Find Nodes?
    Voting Booth?
    Should Pluto Get Its Planethood Back?

    Results (313 votes). Check out past polls.