Community
Participate
Working Groups
Build Identifier: M20110909-1335 We have analyzed the refactoring usage data captured by CodingSpectator <http://codingspectator.cs.illinois.edu> to find the frequent problems that the refactoring tool reports to programmers. We have studied the reaction of programmers to a few problems that the refactoring tool reports. As a result, we have some suggestions for improving the usability of the refactoring tool based on our observations. Reproducible: Always Steps to Reproduce: 1. Invoke the Extract Method refactoring by selecting the statements marked in the code below: ---- public class C { private int m() { int a; int b; // START SELECTION a = 1; b = 2; // END SELECTION return a + b; } } ---- 2. The refactoring tool will report the "Ambiguous return value:..." message and prevent the user from continuing the refactoring.
Created attachment 207687 [details] The Extract Method refactoring tool may report the "Ambiguous return value: ..." problem. If multiple local variables are assigned inside the set of selected statements and used outside the selected statements, the Extract Method refactoring tool will report the following error message and prevent the user from continuing the refactoring: > Ambiguous return value: Selected block contains more than one assignment to local variables. Affected variables are: {0} and {1}.
The Extract Method refactoring is popular and frequently used. CodingSpectator reported that the Extract Method refactoring reported a total of 59 problems of 11 different kinds to our participants. The above error message accounted for 34% of the problems that Extract Method refactoring reported to our participants. This was the most common problem that the Extract Method refactoring tool reported to our participants. Five of our participants received this error message at least once from the Extract Method refactoring tool. We have analyzed the CodingSpectator data to see how our participants have reacted to this problem. We've also asked some of our participants how they generally handled this problem of the Extract Method refactoring tool. We've found that our participants reacted to this problem in the following ways: 1. The programmer just quits the refactoring when the refactoring tool fails to perform it. 2. The programmer removes the uses of the local variables outside the selected statements. 3. The programmer narrows the selection and tries the refactoring tool again to select a different set of statements into a method. 4. The programmer widens the selection and tries the refactoring tool again to select a different set of statements into a method. 5. The programmer converts some of the local variables into fields and tries to extract the same list of statements into a method. 6. The programmer extracts the selected statements into a method and makes the new method return an object whose fields hold the values assigned to the local variables in the selected statements. Eclipse Help <http://help.eclipse.org/indigo/index.jsp?topic=/org.eclipse.jdt.doc.user/reference/ref-refactoring-extract-method.htm> suggests techniques 3 and 4 for this problem. One of the conclusions of our study was that programmers prefer flexible refactoring tools that allow programmers to perform the change even if some pre-conditions fail. Our participants performed refactorings that reported errors (as opposed to warnings or fatal errors) 66% of the time. Our participants' explanation for this behavior was that it was easier for them to perform a possibly unsafe refactoring and fix its outcome than start over, satisfy the preconditions, and reconfigure the tool. For example, one of our participants said: >Usually, I might even like that the tool would do the refactoring and I would continue to adapt the remaining things manually. I kind of don't like the fact that it doesn't allow me to perform the refactoring. Is it possible to make the Extract Method refactoring tool more flexible with respect to the "Ambiguous return value: ..." problem? That is, can the tool still let the programmer perform the refactoring and fix the resulting code manually? Letting the programmers perform an unsafe refactoring might give them a better opportunity to understand the problem. The Extract Method refactoring could go beyond just reporting the problem and suggest some of the techniques we listed above to satisfy the precondition of the refactoring tool. A smart refactoring tool could even propose the programmer to automatically apply one of the fix strategies and let the programmer evaluate the fix and choose the best one. For example, if the programmer decides to try the widening strategy (strategy #4), the tool would automatically extend the selection to cover the uses of the selected local variables. Then, the programmer will reevaluate the refactoring with the new selection.
(In reply to comment #2) > One of the conclusions of our study was that programmers prefer flexible > refactoring tools that allow programmers to perform the change even if some > pre-conditions fail. Our participants performed refactorings that reported > errors (as opposed to warnings or fatal errors) 66% of the time. Our > participants' explanation for this behavior was that it was easier for them to > perform a possibly unsafe refactoring and fix its outcome than start over, > satisfy the preconditions, and reconfigure the tool. For example, one of our > participants said: > > >Usually, I might even like that the tool would do the refactoring and I would continue to adapt the remaining things manually. I kind of don't like the fact that it doesn't allow me to perform the refactoring. In the example from comment 0 I think it is better to simply change the selection and invoke the refactoring again. Do you have examples where it would make sense to allow an unsafe refactoring? > Is it possible to make the Extract Method refactoring tool more flexible with > respect to the "Ambiguous return value: ..." problem? That is, can the tool > still let the programmer perform the refactoring and fix the resulting code > manually? Letting the programmers perform an unsafe refactoring might give them > a better opportunity to understand the problem. Extract Method refactoring allows to change semantics in one case, though you get the following warning first.. "Selected statements contain a return statement but not all possible execution flows end in a return. Semantics may not be preserved if you proceed." But this needs to be done on a case by case basis. Do you have concrete examples? :-) > The Extract Method refactoring could go beyond just reporting the problem and > suggest some of the techniques we listed above to satisfy the precondition of > the refactoring tool. A smart refactoring tool could even propose the > programmer to automatically apply one of the fix strategies and let the > programmer evaluate the fix and choose the best one. For example, if the > programmer decides to try the widening strategy (strategy #4), the tool would > automatically extend the selection to cover the uses of the selected local > variables. Then, the programmer will reevaluate the refactoring with the new > selection. Given the number of alternatives available to the user to fix the problem, I think implementing this would add unnecessary complexity to the refactoring, especially because the error message clearly states which variables cause the problem.
(In reply to comment #3) > In the example from comment 0 I think it is better to simply change the > selection and invoke the refactoring again. Do you have examples where it would > make sense to allow an unsafe refactoring? It's always safer to quit a refactoring that has reported a problem, go back to the source code, change the code and selection to satisfy the preconditions of the refactoring, and invoke the refactoring again. Surprisingly, we've found that this is not how programmers prefer to deal with refactoring problems. Instead, programmers prefer to perform a broken refactoring and fix the resulting code. This is because the overhead of quitting the refactoring and reconfiguring the tool is perceived to be high. The programmer has to understand the error message and its underlying cause, quit the refactoring, make the necessary corrections, invoke, and reconfigure the refactoring. Nonetheless, if the programmer hasn't made the right changes, the refactoring tool might still report a problem. At this point, the value of the refactoring tool begins to diminish. As a result, most programmers prefer to perform a broken refactoring and fix the resulting code manually. This strategy works specially well for refactorings such as Extract Method because they affect a narrow piece of the code and the resulting broken code can be easily fixed. In summary, there are two ways of dealing with a refactoring problem. Sometimes programmers quit the refactoring tool and start over. Other times, programmers preform the refactoring and fix the resulting broken code manually. I think both strategies are reasonable for different people in different situations. Therefore, it would be nice to make the refactoring tool more flexible to support both strategies. The Extract Method refactoring could still report the error message "Ambiguous return value: ..." but also allow programmers to continue the refactoring if they prefer to fix the problem afterwards.
(In reply to comment #3) > (In reply to comment #2) > > The Extract Method refactoring could go beyond just reporting the problem and > > suggest some of the techniques we listed above to satisfy the precondition of > > the refactoring tool. A smart refactoring tool could even propose the > > programmer to automatically apply one of the fix strategies and let the > > programmer evaluate the fix and choose the best one. For example, if the > > programmer decides to try the widening strategy (strategy #4), the tool would > > automatically extend the selection to cover the uses of the selected local > > variables. Then, the programmer will reevaluate the refactoring with the new > > selection. > Given the number of alternatives available to the user to fix the problem, I > think implementing this would add unnecessary complexity to the refactoring, > especially because the error message clearly states which variables cause the > problem. I agree that it is nontrivial to propose automated fixers for the "Ambiguous return value: ..." problem. How about giving some hints to the programmer on how to go about fixing the problem? The following is my attempt to include the main fix strategies in the message: "Selected statements contain more than one assignment to local variables that are used in the following code. Assigned local variables are: int a, int b. The new method can return at most one value. Convert some of the local variables to fields, narrow your selection to exclude some of the assignments to local variables, or widen your selection to include uses of some local variables in the code following the selection."
Bug 365664 is related to this bug.
I have had instances when I have had to perform a refactoring manually because Eclipse would not proceed because of an error. On such occasions I do wish for things to be a bit more flexible. (In reply to comment #3) > Extract Method refactoring allows to change semantics in one case, though you > get the following warning first.. > "Selected statements contain a return statement but not all possible execution > flows end in a return. Semantics may not be preserved if you proceed." Markus, similar to above case maybe we can also allow the refactoring in case of Ambiguous return value, and just return the first local variable. (Of course we will have to duly warn the user and give an option to cancel or continue) i.e. for the example from comment 0 the result could be transformed to private int m() { int a; int b; // START SELECTION a = extracted(); // END SELECTION return a + b; // this line will have an error, but it is ok } private int extracted() { int a; int b; a = 1; b = 2; return a; }
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
