[Cuis-dev] Re : ExtractMethod improvements

[H. Fernandes]

Hi Joel,

I am wondering if it makes sense to allow such refactored code.
For a human, the resulting code is more difficult to understand than the former one; at least for me.


Dr. Geo -- http://gnu.org/s/dr-geo

----- Joel Camera via Cuis-dev <cuis-dev at lists.cuis.st> a écrit :
> Hi everyone,
> 
> I wanted to share some improvements I made to the Extract Method
> refactoring in Cuis. There are three main additions:
> 
> *1. Support for assignments to temporary variables*
> 
> Previously, if you tried to extract a fragment that assigned a temp var,
> the refactoring would fail with an error. Now it handles several cases
> automatically:
> 
> - If the variable isn't used after the fragment, it just moves inside the
> new method.
> - If it's used after, the new method returns it and the call site assigns
> it back.
> - If multiple variables are assigned and all used afterwards, it uses an
> "assignment block" pattern:
> 
> *"Before:"*
> process
> | subtotal tax |
> subtotal := self price.
> tax := subtotal * self taxRate.
> ^ subtotal + tax
> 
> *"After extracting 'subtotal := self price. tax := subtotal * self
> taxRate.':"*
> process
> | subtotal tax |
> self computeAssigningWith: [ :newSubtotal :newTax |
> subtotal := newSubtotal. tax := newTax. ].
> ^ subtotal + tax
> 
> computeAssigningWith: anAssignmentBlock
> | subtotal tax |
> subtotal := self price.
> tax := subtotal * self taxRate.
> ^ anAssignmentBlock value: subtotal value: tax
> 
> *2. Support for non-local returns*
> 
> Extracting fragments with a `^` also used to fail. Now:
> 
> - A plain `^` in the fragment becomes the return of the new method.
> - A `^` inside a block uses a "returning block" pattern so the non-local
> return still works correctly:
> 
> *"Before:"*
> findFirst
> self do: [:each | each isValid ifTrue: [^each]].
> ^ nil
> 
> *"After extracting 'self do: [...]':"*
> findFirst
> self findValidElementReturningWith: [:res | ^ res].
> ^ nil
> 
> findValidElementReturningWith: aReturningBlock
> self do: [:each | each isValid ifTrue: [^ aReturningBlock value: each]].
> 
> 
> *3. Parameterizable code detection*
> 
> Extract Method now detects occurrences that are structurally similar to the
> selected fragment but differ in some arguments — just like it already does
> for equivalent code, it searches across the whole class hierarchy. When
> found, a new window appears letting you decide whether to parametrize them:
> 
> *"Before:"*
> process
> self foo: 10 bar: 'hello'.
> self foo: 20 bar: 'hello'.
> self foo: 10 bar: 'world'.
> 
> *"After selecting 'self foo: 10 bar: ''hello''' and accepting
> parametrization:"*
> process
> self m: 10 with: 'hello'.
> self m: 20 with: 'hello'.
> self m: 10 with: 'world'.
> 
> m: aFoo with: aBar
> self foo: aFoo bar: aBar
> 
> All three improvements are already integrated into Cuis. Thanks Juan for
> merging them!
> 
> Happy to answer any questions!
> 
> <https://10pines.com/>Joel CámeraAgile Software Development
> 
> Alem 896, Floor 6, Buenos Aires, Argentina
> 
> +54 9 011 6091 3125