Beefy Boxes and Bandwidth Generously Provided by pair Networks
Keep It Simple, Stupid
 
PerlMonks  

Refactoring Perl #3 - Inline Temp

by agianni (Hermit)
on Jul 17, 2007 at 19:39 UTC ( #627111=perlmeditation: print w/ replies, xml ) Need Help??

You have a temp that is assigned once with a simple expression, and the temp is getting in the way of other refactorings.

Replace all references to that temp with the expression.

(Fowler, p. 119)

Inline Temp is probably the most trivial refactoring pattern:

my $base_price = $self->an_order->base_price(); return ( $base_price > 1000 );

becomes:

return ( $self->an_order->base_price() > 1000 );

Get the code

This causes occasional consternation in code reviews I attend, as some like to create temp variables to clarify their code and other find it to be unnecessary clutter. I find that programmers most often employ this technique when a verbose method name or deep data structure would otherwise exist multiple times in code:

my $thing = $that_object->that_method->{thing}; $obj->do_this_with( $thing ); $obj->do_that_with( $thing );

I generally avoid getting into scuff-ups about such matters. Fowler suggests this refactoring for situations where the temp gets in the way of other refactoring, generally Replace Temp With Query (up next).

Fowler also suggests an interesting technique to aid in the refactoring:

Declare the temp as final if it isn't already and compile. This checks that the tem is really only assigned to once. (Fowler, p. 119)

While Perl lacks the means to turn a once dynamic variable into a read-only (does anyone know of one?) there are a number of ways to create read only variables in perl. My favorite is Readonly. This is a debugging technique that I hadn't considered before.

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

Comment on Refactoring Perl #3 - Inline Temp
Select or Download Code
Re: Refactoring Perl 3 - Inline Temp
by hossman (Prior) on Jul 17, 2007 at 20:15 UTC

    i'm not sure what you mean by "to turn a once dynamic variable into a read-only" ... that doesn't really seem to relate to the refactoring at hand, nor does it relate to declaring a variable "final" in java ... Readonly seems to be the perfect corollary for perl.

    UPDATE: based on a message i just got, apparently i wasn't clear about my "i'm not sure what you mean" comment ... I do understand the concept of "locking" a variable that could previously be changed so that once it is locked it is effectively readonly -- but i assumed i must have been misunderstanding the OP question, since that has nothing to do with the refactoring.

    This refactoring only works if a variable is assigned once, the comment by Fowler about declaring the variable "final" before attempting the refactoring is all about ensuring that it really is a "read only" from the very start.

      Sorry, I think the first part of the problem was my misunderstanding of the final keyword in Java. I didn't realize that it was a declaration keyword. As such, something like Readonly is a perfect equivalent in Perl (although the additional recommendations are much appreciated).

      As for the second issue, this doesn't have anything to do directly with refactoring. I simply pointed out that Fowler suggested this method for a sanity check before performing the refactor. I just thought this was useful enough to call out, much as I have mentioned testing techniques in other write-ups that don't directly relate to refactoring.

      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));'
Re: Refactoring Perl 3 - Inline Temp
by ikegami (Pope) on Jul 17, 2007 at 20:18 UTC

    While Perl lacks the means to turn a once dynamic variable into a read-only (does anyone know of one?)

    For SVs, Scalar::Readonly. It's basically identical to Readonly::XS (whose interface is private and undocumented). It would be nice if it was part of Scalar::Util which currently has a function to check the read-only flag (readonly), but none to set it.

    I don't see anything on CPAN to make AVs or HVs read-only. It could be that Perl doesn't support read-only AVs and HVs. I'll have to check that when I get home. (Readonly fakes it by tieing magic unto variables.)

Reaped: Re: Refactoring Perl 3 - Inline Temp
by NodeReaper (Curate) on Jul 17, 2007 at 20:34 UTC
Re: Refactoring Perl 3 - Inline Temp
by BrowserUk (Pope) on Jul 17, 2007 at 20:34 UTC
Re: Refactoring Perl 3 - Inline Temp
by rvosa (Curate) on Jul 21, 2007 at 16:23 UTC
    So, what if the method is expensive? Why would you want to do the same thing twice?

      You certainly don't have to refactor. This is just a pattern you can use to make your code cleaner. I generally find that writing good code is always a balance between clean code and efficient code. It's great when you can do both, but often you have to pick one.

      If you really need this refactoring and the method is expensive, you can always look into caching mechanisms. I can't recommend anything particularly complicated, as I haven't done any caching of methods that take arguments (the complicated part) but I'm sure there are some CPAN modules that would help.

      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?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlmeditation [id://627111]
Approved by Corion
Front-paged by adrianh
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others chanting in the Monastery: (7)
As of 2014-08-21 22:59 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    The best computer themed movie is:











    Results (144 votes), past polls