Re: "Replace Temp with Query" use cases.

On Sat, Nov 7, 2009 at 4:17 PM, mszerg <[email protected]> wrote:
> Hi All,
>
> Recently I faced the following situation. In our project we have
> several functions with php code like this:
>
> // some global function
> function doSomethingWithX(X $x) {
> $x->getY()->setA(..);
> $x->getY()->setB(..);
> $x->getY()->setC(..);
> ...
> $x->getY()->setZ(..);
> }

That's a lot of repetition there.

> During code review I suggested the following refactoring:
>
> function doSomethingWithX(X $x) {
> // store reference locally
> $y = $x->getY();
> // modify fields
> $y->setA(..);
> $y->setB(..);
> $y->setC(..);
> ...
> $y->setZ(..);
> }

That looks good to me.  It's what I would have done, given the same code.

> In my opinion it is just simpler, not saying 2x faster. But the authors
> say that "...according to Fowler's "Replace Temp with Query" refactoring
> strategy our version is better". From my conversations with the other
> colleagues, this looks like common approach. I suppose that this can be
> misunderstanding of the strategy use cases, but I may be wrong. Please,
> explain me what is right and wrong in this situation.

I don't have my copy of Fowler in front of me, and can only check
what's on the refactoring.org web site.  I am very surprised to find
that your refactoring (which ReSharper calls "Introduce Variable") is
not in his list.  He does appear to have two different refactors for
the inverse of your refactoring: "Replace Temp with Query" and "Inline
Temp".  From the looks of it, Fowler doesn't like temporary variables.

I don't think Replace Temp with Query is appropriate.  In Fowler's
canonical example, he essentially performs two simpler refactorings:
Extract Method, to get a new method which calculates the value of the
temp, and Inline Temp, to replace usages of the temp with calls to the
new method.  In the refactoring from your form (with $y = $x->getY())
to your author's form (with all the $x->getY()->setX() calls), there
is no extracting of a method to compute the value of the temp.  There
is just the Inline Temp part.

Inline Temp by itself is also problematic.  The description of Inline
Temp Fowler gives is: "You have a temp that is assigned to once with a
simple expression, and the temp is getting in the way of other
refactorings."  I don't see your $y as getting in the way of other
refactorings.

Which version of the code is "better" is always a judgment call.
Since a refactoring is a change in the design of the code which does
not affect behavior, then almost by definition the inverse of any
refactoring is also a refactoring.  Sometimes these inverse pairs are
both listed in Fowler's book (example: Extract Method/Inline Method).
ReSharper calls what you did "Introduce Variable", Using Fowler's
terminology, I'd call it either "Extract Common Expression" or
"Replace Common Expression with Temp".  And in this case, it would be
a good thing.

> --
> Thanks in advance,
>  Serg Masyutin


------------------------------------

Yahoo! Groups Links

<*> To visit your group on the web, go to:
    http://groups.yahoo.com/group/refactoring/

<*> Your email settings:
    Individual Email | Traditional

<*> To change settings online go to:
    http://groups.yahoo.com/group/refactoring/join
    (Yahoo! ID required)

<*> To change settings via email:
    [email protected] 
    [email protected]

<*> To unsubscribe from this group, send an email to:
    [email protected]

<*> Your use of Yahoo! Groups is subject to:
    http://docs.yahoo.com/info/terms/