[Eap-features] Re: Refactory: Replace Inheritance with Delegation

[Jacques Morel]

> > Move method
>
> It is quite easily performed with the following refactorings: make method
> static, move static method, introduce parameter. The reason why move method
> is not implemented is simple: the amount of information, IDEA should ask a
> user when performing the method is _enormous_. So it won't be very practical
> to use. (Try thinking about several possibilities in moving the method).
>

However it would be nice to add support in Move Static Method to fix the errors it warns about right in the dialog (a la Inspect Code tool). It should provide an option to immediately.

1. encapsulate the fields the method directly accesses and use public setter/getters instead.
2. change the visibility of methods that the method would no longer have access to.

At the very least 1. should have the same options as Introduce Parameter (change direct access to accessors when not accessible). In addition all refactorings should be smart enough to remember their settings. That way if it cannot go through for any reason (impossibility, user needs to do something before) the user won't have to start over.

 

Anyway, converting to static as a way to break data dependencies seems artificial to me. When a method smells of Feature Envy I want to move to its new class, not make it static. I will have a hard time to train my other users. We need something that speaks to a novice especially on such crucial refactoring.

 

So let's go back to what Move Method needs to do:

1. Ask the user what field or parameter should the method be moved to. 
2. Ask to change the visibility of any method called by the method to be moved that is not visible from target object.
3. Ask to encapsulate any field accessed by the method to be moved that is not visible from target object.
4. At call site create a method that delegates to the moved method (that way callers are not affected. You can always inline it later).
5. Introduce a parameter for the old object in the moved method.
6. In body of the moved method qualify any access to the old object with the new parameter.
7. In body of the moved method remove any explict reference to the target object. They are now implicit.

Am I missing something here? That doesn't seem too big. Especially what moving a method right now requires:

1. Move method to a field class

2. Make method static

3. Introduce parameter for the target field if necessary

4. Make all methods accessed visible from destination class

5. Encapsulate all fields that method accesses.

6. Move method

7. At call site change class qualification with target field/parameter

8. At method declaration remove static

9. Remove parameter usage in method

10. Remove parameter in method.

I will always forget these steps. It would be nice if IDEA would help me along the way.

 

> > Extract Class/Inline Class
> This is even more complex than Move method.

As defined in the Refactoring book it is a set of Move Method and Move Field. I guess we really need Move Method and add Move Field (of non static members) to the mix.
>
> > Replace Inheritance with Delegation
> Sometimes we are thinking about having this refactoring too.

It might be a little easier than move

>
> > Replace Constructor with Factory Method
> Are you performing it often?

This is a first step to pluggability usually. After this refactoring is done there is only one method you need to change in order to vary which derived class you want to instantiate. Obviously you need to do more but this is potentially the most labor intensive step. I use this kind of pattern often when testing legacy code (code with no tests) with mock objects. I need a way to insert my mocks. A factory is usually a temporary solution, a lesser evil to introduce the mocks with the least impact.

 

>
> > Encapsulate Collection (mentioned several times in other threads)
> It's quite impossible to transform setCourse(Set) method to add/removeCourse
> automatically. Or are we missing something?

I guess you are right. It is a generate action not a refactoring, like setter and getter.

>
> > Replace Parameter with Method
> Isn't this easily performed with Inline Variable?

No this one is from the call site. It is the reverse of Introduce Parameter. It should be renamed Delete Parameter

Introduce Parameter move a variable initialization from the callee to the caller

Delete Parameter move it back from the caller to the callee and remove the parameter.