[Pharo-project] Refactoring Questions
after having a look with ben at the current refactoring implementation in Nautilus we found that the "Extract into temporary" refactoring is most of the time not that useful. I wonder wether it would make sense to loosen the requirements on conserving behavior... consider the following example where we extract "self foo" into the temporary "tmp": =========================================== foo ^ self foo even ifTrue: [ self foo * 2 ] ifFalse: [ self foo / 2 ] actual result ============================= foo | tmp | tmp := self foo. ^ tmp even ifTrue: [ self foo * 2 ] ifFalse: [ self foo / 2 ] proposed result =========================== foo | tmp | tmp := self foo. ^ tmp even ifTrue: [ tmp * 2 ] ifFalse: [ tmp / 2 ] =========================================== note that the proposed solution is not side-effect free. However the given refactoring hardly makes ever sense, since this action can be easily done manually in less time. given that you first have to find the proper menu-item. best cami
Why not have a separate "edit" called e.g. eliminate common sub-expression? Preserve the true refactoring and add an edit that searches for all occurrences of the selected sub-expression, prompting for a temp var? The fact that refactorings are safe is important. That doesn't make other modifications useless, but it doesn't make them refactorings either :) On Thu, Feb 16, 2012 at 10:25 AM, Camillo Bruni <camillobruni@gmail.com>wrote:
after having a look with ben at the current refactoring implementation in Nautilus we found that the "Extract into temporary" refactoring is most of the time not that useful.
I wonder wether it would make sense to loosen the requirements on conserving behavior...
consider the following example where we extract "self foo" into the temporary "tmp": =========================================== foo ^ self foo even ifTrue: [ self foo * 2 ] ifFalse: [ self foo / 2 ]
actual result ============================= foo | tmp | tmp := self foo. ^ tmp even ifTrue: [ self foo * 2 ] ifFalse: [ self foo / 2 ]
proposed result =========================== foo | tmp | tmp := self foo. ^ tmp even ifTrue: [ tmp * 2 ] ifFalse: [ tmp / 2 ] =========================================== note that the proposed solution is not side-effect free.
However the given refactoring hardly makes ever sense, since this action can be easily done manually in less time. given that you first have to find the proper menu-item.
best cami
-- best, Eliot
Hi there, Any news about that improvement? I seconded Camillo. Cheers Davide -- View this message in context: http://forum.world.st/Refactoring-Questions-tp4394983p4632209.html Sent from the Pharo Smalltalk mailing list archive at Nabble.com.
participants (3)
-
Camillo Bruni -
Davide Varvello -
Eliot Miranda