Refactoring proposal: move (non-static) methods to a helper class

It's currently possible to move methods to a superclass, or even create a new superclass with selected methods. How about providing the same thing, but without the class hierarchy? If most people coding patterns are similar to mine, this would be imensely useful. It goes like this...

Suppose I have to code some complex functionality. On a first step, I'll just write down what the code is supposed to do, on a huge method. On a second step, I'll break up this big method on smaller, more focused and easier to understand methods, and thankfully IDEA makes this a breeze. On a third step, I find out that a number of the extracted methods aren't exactly core concerns and could be moved to a helper class. Data access usually falls in this category.

Unfortunately, IDEA doesn't help me on this first step. I have to create the helper class manually, copy the methods over to the new class, and create a depenency on it in the old class. It would be really, really nice if IDEA could do this for me.

Any thoughts?

12 comments
Comment actions Permalink

Marcus

>Unfortunately, IDEA doesn't help me on this first step. I have to create the helper class manually, copy the methods over to the new class, and create a depenency on it in the old class. It would be really, really nice if IDEA could do this for me.
>

>


When moving a method, if the target class doesn't exist, IDEA will offer
to create it for you.

Alain

0
Comment actions Permalink

I tend to make such methods static and then move them off to helper classes. There's currently a 'Make Method Static'-refactoring, and you can move static members.

Not suggesting that your proposal be rejected on those grounds, but you can take that path now.

0
Comment actions Permalink

This only works for static methods. Maybe helper class was a misnomer on my part. I'm talking about breaking up a current class by indentifying and extracting smaller components which can be configured using standard dependency injection mechanisms. These smaller components will sometimes have state of their own, precluding the use of static methods.

Think about the usual "business service" and "data access object" pattern. When I start coding, everything will be in the same place (what will be the business service when everything is done). As I further refactor the code, the methods suitable to be moved to a DAO will stand out. Now, the DAO itself will have some state, like a reference to a DataSource object. This instance variable was previously stored on the business service, and will be moved together with the relevant methods to the DAO.

After the DAO is extracted (manually, for now), I'll declare a new instance variable on the business service, typed as the DAO, and provide a setter or a constructor argument to configure it.

Am I asking too much? This seems to be a common pattern nowadays, with the advent and popularity of IoC frameworks like Spring and Hivemind. It would be very nice if IDEA could help me a little here.

0
Comment actions Permalink

Marcus

>This only works for static methods.
>


If the 2 features below were implemented, what would you be missing :


(I keep repeating those 2 sequences again and again. I wish IDEA would
offer real macros, but that's another story)

seq 1: make static + move
seq 2: make static + Convert to instance method


seq 1:
Very often, I do "seq 1" because IDEA wouldn't move a non-static method,
even if it could be made static at no cost (no parameter to pass, just
press enter).

I wouldn't have to do that if:
"[F6] on non static method : ok if can be made static at no cost. "
http://www.intellij.net/tracker/idea/viewSCR?publicId=18762


seq 2:
As above, most of the time, IDEA could check if, and automatically make
the method static,
"Make static + Convert to instance method: combine in 1 action"
http://www.intellij.net/tracker/idea/viewSCR?publicId=39784


Alain

0
Comment actions Permalink

Here's what would cover my use case:

1) Move (a set of) non-static methods to another (possibly new) class, along with any related instance variables. IDEA should check that the instance variables are only used by the methods being moved, and automaticaly suggest to move instance variables that match this criteria. This action could be named "Split Class" or something like that.

2) Introduce a dependence on the newly created class, by automatically creating a instance variable and a setter method (or a constructor argument, and the user's discretion).

I would be happy with (1) alone, (2) is just would just be added bonus (that would get good press releases for IDEA on IoC communities).

0
Comment actions Permalink

Marcus

>1) Move (a set of) non-static methods to another (possibly new) class, along with any related instance variables. IDEA should check that the instance variables are only used by the methods being moved, and automaticaly suggest to move instance variables that match this criteria. This action could be named "Split Class" or something like that.

>


If IDEA would allow the user to trigger "Convert to instance method" on
non-static methods, this would be 1 step in your direction.
The second step would be to allow this on multiple methods at the same
time, the same way as "Make method static" does.
That would be great.

It reminds me a request by Jacques Morel, but I could be wrong.


Alain

(note : step 1 is
"Make static + Convert to instance method: combine in 1 action"
http://www.intellij.net/tracker/idea/viewSCR?publicId=39784
)

0
Comment actions Permalink

Doesn't "Convert to instance method on non-static methods" sounds weird? After all, if the method is non-static, it's an instance method already! Or am I missing something?

0
Comment actions Permalink

Marcus Brito wrote:

>Doesn't "Convert to instance method on non-static methods" sounds weird? After all, if the method is non-static, it's an instance method already! Or am I missing something?

>


It's just the merging of the sequence ("Make static" + "convert.. to
instance method") into 1 action.
Nothing weird here.

As I wrote earlier, I keep repeating those actions in sequence. And on
MacOS, it's a pain, as menu shortcuts don't work: you need your mouse to
do basic things as pressing the "Do Refactor" button, toggling
checkboxes in refactoring dialogs, etc...

Alain

0
Comment actions Permalink

I was thinking about the proposal a bit more. The new refactoring interface would be more like the current "Make method static" interface, but instead of selecting fields to change into parameters, you would select fields to be moved together with the class.

On a second thought, this may be tricky: how to initalize the selected fields on the new class? I, as a human being, am able to answer this question easily, but unfortunately not on a deterministic manner. A quick shot at it would be:

If the selected fields are constructor parameters to the current class, it should also be a constructor parameter to the target class. If the selected field has a setter on the current class, it should also has a setter on the target class. This would cover the 90% of the cases where the field is just there for dependency injection. On a second step, IDEA should migrate the field initialization calls: search for the current class constructor calls and replace remove the said parameter, substituting it with a call to the target class constructor; search for the current class field setter and substitute it with a call to the target class field setter.

... On a third thought, this is getting too complicated to automate without breaking anything. I was so used to doing this all the time that I didn't realize the inherent complexity in this apparently simple refactoring. Maybe it's time for me to start thinking about writing a plugin to do this myself.

0
Comment actions Permalink

Marcus,

>... On a third thought, this is getting too complicated to automate without breaking anything. I was so used to doing this all the time that I didn't realize the inherent complexity in this apparently simple refactoring.
>


Could you give an example - before and after - of the simplest case? And
then an example a little more complex .


Alain

0
Comment actions Permalink

Not sure if I'll be able to picture this in a simple enough to understand without pasting (long streams of) code, but it works like this:

Suppose you get a RFI that asks for some reasonably complex new operation on your system. Thankfull, the RFI already contains a correct and detailed description of how this is supposed to work. To use real world examples, yesterday I was supposed to write the code to plan (distribute among a sequence of time periods) SMS broadcast actions on a cell phone company.

So, I start writing the planBroadcast(...) method. The algorithm isn't too complicated, actually, but involves lots of steps.

At first, I don't think how to structure the code, just write it down as fast as I can. In the end, I have a class (say, BroadcastPlanner) with a single 1000 lines method, and a couple of instance variables: references to other services in the same system, and a JdbcTemplate[/url] instance. These variables are dependencies to my class, and they will be configured automatically by the Spring ApplicationContext as long as I provide setters to them.

Second step, I go thru the method I just wrote, and break it down on many smaller methods, using the "Extract Method" refactoring. Now my code is easier to understand, better structured, and someone else can almost make sense out of it. I have just one problem now: a number of the extracted methods are there only for accessing the database, while the rest of the methods contains valid business logic.

The third step is to separate the database calls from the business logic. I create a new class (say, BroadcastPlannerDAO) and move all the methods that are purely data access operations to this new class. These methods need to access the JdbcTemplate previously mentioned, so I have to move it too, and provide a setter to it. On the old class, I declare a new instance variable typed as BroadcastPlannerDAO, and provide a setter to it.

That's it, my work is done. Now I have a BroadcastPlanner class with my core business logic and a BroadcastPlannerDAO, with my data access operations. The former has a dependency on the latter, on the form of a setter method. This dependency will be automatically configured by Spring on the system initalization.

The only part that IDEA doesn't help me now is the third step described above: the creation of the BroadcastPlannerDAO class, and moving the methods from BroadcastPlanner to it. The methods being moved aren't static on the current class, won't be static on the new class, and they do depend on a instance variable. On a perfect world, I would select the methods and tell IDEA to move them to BroadcastPlannerDAO. It would detect that they depend on the JdbcTemplate instance variable, and ask to move that too.

So, was this example clear enough? Does my usage pattern makes any sense at all?

0
Comment actions Permalink

I agree. I think resharper could advice / help the developer to identify the "de facto" static method and warn the developer if most of his method are static.
A static method is actually a method that doesn't use field of tha class which defines it. If a lot of methods are static, the class could be defined as static itself. If this happens the class is an Helper or the developer has not understood the object model.

Resharper could gap this.

0

Please sign in to leave a comment.