Summary: | [extract method] creates un-needed code | ||||||
---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | Gary Gregory <ggregory> | ||||
Component: | UI | Assignee: | Benjamin Muskalla <b.muskalla> | ||||
Status: | NEW --- | QA Contact: | |||||
Severity: | enhancement | ||||||
Priority: | P3 | CC: | b.muskalla, markus.kell.r | ||||
Version: | 3.0 | ||||||
Target Milestone: | --- | ||||||
Hardware: | PC | ||||||
OS: | Windows XP | ||||||
Whiteboard: | |||||||
Attachments: |
|
Description
Gary Gregory
2004-05-26 18:30:36 EDT
Move to JDT/UI Not critical for 3.0. More compact example: public class A { public void foo() { Object o= null; String s= null; s= (String)o; System.out.println(s); } } Created attachment 143217 [details] sketch Markus, here is a first sketch how this could look like. The patch still needs some cleanup. Currently there are 16 failing tests because of this simplification. Just didn't want to touch the exisiting testcases before we agree on a patch. While the patch allows the simplification mentioned in comment #0 we would need to introduce a regression regarding comments in the original code. See this snippet which shows the regression: public class Snippet { public void f() { int x; x= 20; // extract the whole line System.out.println(x); } } With this patch we cannot move the comment to the new extracted method. Just want to check back with you if it's worth. We must not lose the user's comments unless that's forced by the refactoring (e.g. you know you will lose the Javadoc of a method you inline). I know we already violate this in a few places, but we really need to go the extra mile to ensure this doesn't happen. The approach you've taken looks good. To preserve the comments, here's my current idea (but I haven't tried this, so it might not work at all): Instead of moving just the RHS of the assignment, you could try to move the whole ExpressionStatement. Before you move the ExpressionStatement, replace its contents with the new VariableDeclarationStatement. The key of the idea is that you also tweak the SelectionAwareSourceRangeComputer and tell it that it should only replace the tightest possible range (getStartPosition()+getLength()) of the ExpressionStatement, so that the comments stay and can be moved with the node. Nit on source code formatting: Note that we always have a space between keyword and parenthesis, so "if(expr)" is wrong. |