please consider enhancing this feature

i use "extract method" quite often. in most cases, i have a simple line like
a = calcSomething(d.getX());

if i extract "calcSomething(d.getX());", there will just be another method delegating a call looking like this: a = newMethod(d);
but if i extract a local variable first (tmp = d.getX()). idea will suggest to replace all spots where "calcSomething(anything having the return type of tmp)" is called with "newMethod(anything having the return type of tmp)". this is often what i actually want to do - add another "layer". it shouldn't be too hard to enhance extract method to automatically suggest that.

9 comments
Comment actions Permalink

I would love to see that too. I'm not sure how it would be implemented though.  When you extract the method, wouldn't idea have to infer the parameters? I wouldn't expect IDEA to be able to do that.  For example:

public void temp() {
return "select * from " + "myTable" + "where id=" + "1";
}

If I extract the contents of this method into another private method, I may want to parameterize the table name or the id.  The only way to be sure idea would get it right, is to make a second step where the user chooses the parameters from the code.  Really, that's no different than extracting the variable first and then extracting the method.

Another solution would be to extract the method, then extract the parameters, and then kick off IDEA's dupe checking through some sort of intention.  Of course that doesn't save any work from extracting the parameters first, but I don't think there's a way around that, and this would be more obvious than extracting the parameters before extracting the method.  

0
Comment actions Permalink

to make a concrete suggestion:
let the user choose which parameters should be "extracted" by clicking on some checkboxes right below wherer you enter the methods name, defaulting to none

0
Comment actions Permalink

It seems that would be complicated.  Even with the simple example I had above, you could extract a method dozens of different ways:

16 different simple combinations
newMethod("select * from ")
newMethod("select * from ", "myTable")

newMethod("select * from ", "myTable", "where id=")

newMethod("select * from ", "where id=")
etc.


Then you take into consideration that multiple literals could make a single parameter, and you have an even larger set.  An example would be this single parameter method:
newMethod("select * from" + "myTable")

Imagine if the extracted method were a little more complicated.

The popup dialog would almost have to allow the user to highlight code blocks from the method to extract as parameters.

0
Comment actions Permalink

i see no problem here. in this case, the dialog would offer 4 checkboxes, one for every extractable code-atom. all combinations can be done with these checkboxes. also, merging literals is a special case. finding the code-atoms isn't that difficult (smart introduce does something similar)

0
Comment actions Permalink

So parameters could only consist of the smallest extractable block of code?

Only combinations of these 4 parameters would be possible:
"select * from "
"myTable"
"where id="
"1"

But it wouldn't be possible to have a single parameter to represent any of these:
"select * from " + "myTable"
"select * from " + "myTable" + "where id="
"myTable" + "where id="
"myTable" + "where id=" + "1"
"where id=" + "1"

With just four checkboxes, you wouldn't be able to extract a method like this:

private String existingMethod() {
   return newMethod("where id=" + "1");
}

private String newMethod(String whereClause) {
   return "select * from " + "myTable" + suffix;
}

0
Comment actions Permalink

no, 2^4 would be possible. remember, there would be checkboxes, not radiobuttons.

0
Comment actions Permalink

I don't think I'm getting my point across well.  We need to be able to distinguish between these two options:
newMethod("where id=", "1");
newMethod("where id=" + "1");

The first takes two parameters.  The second takes just one, but it's filled with an expression larger than the smallest extractable atomic unit of code.

0
Comment actions Permalink

imho this is a special case, not the standard one

0
Comment actions Permalink

Hello HamsterofDeath,

There is a checkbox: fold parameters. It becomes visible if IDEA can suggest
you to fold parameters into one. It is selected by default for simple cases

E.g. for this code

int [] a = new int[0];
int idx = 0;


IDEA would suggest you to use one parameter (int) instead of two (int[] a,
int idx): newMethod(a[idx]);

I suspect that this feature can be extended so your feedback is appreciated
:)

Thank you
-


Anna Kozlova
JetBrains Inc.
http://www.intellij.com
"Develop with pleasure!"

i use "extract method" quite often. in most cases, i have a simple
line like a = calcSomething(d.getX());

if i extract "calcSomething(d.getX());", there will just be another
method delegating a call looking like this: a = newMethod(d);

but if i extract a local variable first (tmp = d.getX()). idea will
suggest to replace all spots where "calcSomething(anything having the
return type of tmp)" is called with "newMethod(anything having the
return type of tmp)". this is often what i actually want to do - add
another "layer". it shouldn't be too hard to enhance extract method to
automatically suggest that.

---
Original message URL:
http://www.jetbrains.net/devnet/message/5245392#5245392



0

Please sign in to leave a comment.