Community
Participate
Working Groups
Extract Method Refactoring creates un-needed code Version 3.0.m9 In the method: public void test_TestAmp_compFields_cpy() throws ParseException { CobolRecordDefinition rec = getRecordDefinition(); Assert.assertNotNull(rec); CobolComp3FieldDefinition data3 = getData3Field(rec); validateField(data3, 7, 4, false, false); CobolComp3FieldDefinition data4 = (CobolComp3FieldDefinition) rec.getElementNamed("DATA-4"); validateField(data4, 10, 6, false, false); Assert.assertEquals(10, rec.getLength()); } I select: CobolComp3FieldDefinition data4 = (CobolComp3FieldDefinition) rec.getElementNamed("DATA-4"); Then Alt-Shift-M, enter "getData4Field" for the name and pick "public" access. The generated method is: public CobolComp3FieldDefinition getData4Field(CobolRecordDefinition rec) { CobolComp3FieldDefinition data4 = (CobolComp3FieldDefinition) rec.getElementNamed("DATA-4"); return data4; } As opposed to: public CobolComp3FieldDefinition getData4Field(CobolRecordDefinition rec) { return (CobolComp3FieldDefinition) rec.getElementNamed("DATA-4"); } I suppose I can instead select: (CobolComp3FieldDefinition) rec.getElementNamed("DATA-4"); But it is easier to select a whole line.
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.
