This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 224512 - Refactoring: Extract methods from ifelse-branches
Summary: Refactoring: Extract methods from ifelse-branches
Status: RESOLVED FIXED
Alias: None
Product: java
Classification: Unclassified
Component: Hints (show other bugs)
Version: 7.3
Hardware: PC Windows 7
: P3 normal (vote)
Assignee: Jan Lahoda
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-05 00:18 UTC by java_dev
Modified: 2013-01-08 02:26 UTC (History)
1 user (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
proof_1 matched pair (30.57 KB, image/png)
2013-01-07 00:23 UTC, java_dev
Details
refactoring invoked, target highlighted (123.65 KB, image/png)
2013-01-07 00:30 UTC, java_dev
Details
proof_4 - recursive method created (9.11 KB, image/png)
2013-01-07 00:31 UTC, java_dev
Details

Note You need to log in before you can comment on or make changes to this bug.
Description java_dev 2013-01-05 00:18:33 UTC
Here is the method I am trying to create via extract method:


 private String[] createCheckboxesForMultipleClasses(Collection<? extends GuiUtilsProvider> providers, String[] chkBoxIDs)
    {
        chkBoxIDs = createCheckboxesForMultipleClasses(providers, chkBoxIDs);
        return chkBoxIDs;
    }

Netbeans wrote this. All I provided was the name of the method and the seection of code to be replaced. 

As you can see, it's a recursive call to itself. That's just wrong. The return value is also something NB thought up for no apparent reason. 

What NB should have done is created a method named what I named it with a body which is what I had highlighted at the time I used extract method. 

Beats me what is going on. It used to work just fine. Not sure if I've used Extract Method with *this* very download of the IDE or not. 

Here's your "about" :


Product Version: NetBeans IDE 7.3 Beta 2 (Build 201211062253)
Updates: NetBeans IDE is updated to version , NetBeans 7.3 Beta 2
Java: 1.7.0_05; Java HotSpot(TM) 64-Bit Server VM 23.1-b03
Runtime: Java(TM) SE Runtime Environment 1.7.0_05-b06
System: Windows 7 version 6.1 running on amd64; Cp1252; en_US (nb)
User directory: C:\Users\bass\AppData\Roaming\NetBeans\7.3_userdir_extra
Cache directory: C:\Users\bass\AppData\Roaming\NetBeans\7.3_userdir_extra\var\cache

HTH
Comment 1 java_dev 2013-01-05 18:44:55 UTC
Here is the target behavior in pseudocode. It just treats the two parts of the if then else statements as statements unto themselves. 



original code:
.
.
.
if  (condition)
{
do some huge thing ...blah blah blah for 200  lines
}
else 
{
do some equally huge thing for 200 lines blah blah blah
}
.
.
.

Goal  state after  refactor

if (condition)
 processCondition()
else
processNonCondition();



private void processCondition()
{
do some huge thing ...blah blah blah for 200  lines
}

private void processNonCondition()
{
do some equally huge thing for 200 lines blah blah blah
}
Comment 2 java_dev 2013-01-05 18:57:29 UTC
Pretty sure this *ought* to work as I described in the RFE. If-else statements are only nominally one statement. Semicolons define statement boundaries in Java AFAIK. 

Thus, this is two statements:

if (true); else;

and statements are the atomic units that extract-method refactoring operates upon. This is just my POV based on my understanding.
Comment 3 Jan Lahoda 2013-01-06 21:52:06 UTC
I tried to reproduce, but without any luck so far. It would be awesome if you could specify some kind of steps to reproduce and reopen. Thanks.
Comment 4 java_dev 2013-01-07 00:23:59 UTC
Created attachment 129938 [details]
proof_1 matched pair

matched pair of parens indicating statement bounds.
Comment 5 java_dev 2013-01-07 00:29:10 UTC
Hey having some trouble attaching images without losing this description so here is the description. It refers to attached images. The images are going to have to follow. 

Below are the steps to reproduce the bad behavior. 

Note that upon reading your response just now, I tried to create a very simple test class that showed the behavior and failed; I actually got a proper refactoring for an if-else statement in my simple test case. 

Therefore, it may not be an issue with very simple if-else statements or it may not be localized to if-else statements at all and that characterization of this behavior is just a red herring. 

At any rate, here are the steps to reproduce the bad behavior:


Go to Netbean's source and locate the file 

CommonTestsCfgOfCreate 

in package 

org.netbeans.modules.gsf.testrunner.



In that file, go to method  

private Component createCodeGenPanel()



The third statement there begins with :

  if (multipleClasses) {            
            for (GuiUtilsProvider provider : providers) {
                chkBoxIDs = new String[]{
                    provider.getCheckboxText("CHK_PUBLIC"),
                    provider.getCheckboxText("CHK_PROTECTED"),
                    provider.getCheckboxText("CHK_PACKAGE"),
                    provider.getCheckboxText("CHK_PACKAGE_PRIVATE_CLASSES"),
                    provider.getCheckboxText("CHK_ABSTRACT_CLASSES"),
                    provider.getCheckboxText("CHK_EXCEPTION_CLASSES"),
                    provider.getCheckboxText("CHK_SUITES"),
                    provider.getCheckboxText("CHK_SETUP"),
                    provider.getCheckboxText("CHK_TEARDOWN"),
                    provider.getCheckboxText("CHK_BEFORE_CLASS"),
                    provider.getCheckboxText("CHK_AFTER_CLASS"),
                    provider.getCheckboxText("CHK_METHOD_BODIES"),
                    provider.getCheckboxText("CHK_JAVADOC"),
                    provider.getCheckboxText("CHK_HINTS")
                };
                break;
            }
        } 

The first and last bracket above for a matched pair. This is shown by attached image: "proof_1".



Highlight the code between and including that matched pair and invoke the refactoring. 

The refactoring accepts that statement as a valid target for refactoring and a dialog appears. This is shown by attached image proof_3. 

Type in the name myMethod into the refactoring dialog and hit it. 

Result:
The highlighted target statement remains in place, unchanged.

The method myMethod() is created with appropriate parameters and otherwise as described in my bug report- a recursive call to itself and a return value of checkboxIDs . This is shown in image: proof_4.


HTH
Comment 6 java_dev 2013-01-07 00:30:18 UTC
Created attachment 129939 [details]
refactoring invoked, target highlighted
Comment 7 java_dev 2013-01-07 00:30:44 UTC
Comment on attachment 129939 [details]
refactoring invoked, target highlighted

proof_3
Comment 8 java_dev 2013-01-07 00:31:24 UTC
Created attachment 129940 [details]
proof_4 - recursive method created
Comment 9 markiewb 2013-01-07 06:00:20 UTC
Reopened, new reproduction details available
Comment 10 Jan Lahoda 2013-01-07 11:24:27 UTC
Thank you for the test case, should be OK after this patch:
http://hg.netbeans.org/jet-main/rev/72e0ed3d5676
Comment 11 Quality Engineering 2013-01-08 02:26:09 UTC
Integrated into 'main-golden', will be available in build *201301080001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/72e0ed3d5676
User: Jan Lahoda <jlahoda@netbeans.org>
Log: #224512: better handling of blocks being turned into new methods