| Author |
 Topic  |
|
|
legalize
Tomato Guru
USA
119 Posts |
 Posted - Jun 16 2014 : 11:04:12 AM
|
See test suite at http://d3dgraphicspipeline.codeplex.com/releases/view/39824
void ConstOperation() const
{
// #TEST#: EM19 Extract Method on next line
int x = Function1() + Function2();
int y = x*2;
}
Extract Method EM19 creates:
void ConstOperation() const
{
// #TEST#: EM19 Extract Method on next line
MyMethod();
int y = x*2;
}
void MyMethod() const
{
int x = Function1() + Function2();
}
The correct code should be:
void ConstOperation() const
{
// #TEST#: EM19 Extract Method on next line
int x = MyMethod();
int y = x*2;
}
int MyMethod() const
{
return Function1() + Function2();
}
Extract Method is correct if only the initializer for x is extracted. |
http://legalizeadulthood.wordpress.com |
|
|
feline
Whole Tomato Software
United Kingdom
19024 Posts |
Posted - Jun 16 2014 : 2:55:02 PM
|
This is currently a known limitation, and is documented here:
http://docs.wholetomato.com/default.asp?W155
When extracting the declaration of a local variable, VA expects you to extract all uses of that local variable. This is not an easy problem to solve in general, so as a first step we want to detect and warn when we see this problem coming:
case=2063
Its always going to be possible to select a block of code that does not make any sense to extract, so sometimes telling the user about the problem is the best we can do. |
zen is the art of being at one with the two'ness |
 |
|
|
legalize
Tomato Guru
USA
119 Posts |
Posted - Jun 16 2014 : 11:06:36 PM
|
| I disagree with the idea that it's "always going to be possible to select a block of code that does not make any sense to extract", unless you are referring to the limitations of VAX. VAX just introduces the wrong transformation for the selected code here because it doesn't do any data flow analysis between the selected code and the surrounding block. That's the bug. |
http://legalizeadulthood.wordpress.com |
|
|
|
feline
Whole Tomato Software
United Kingdom
19024 Posts |
Posted - Jun 17 2014 : 09:30:10 AM
|
I fully agree there is a problem here, and this is something we want to fix.
But to take a very simple example, what if someone selects half a symbol name? Mismatched numbers of brackets? What happens when you start looking at code with #if #else #endif or macros that introduce brackets?
Its really easy to think up selections that don't make sense to extract. Now most of the time this will be a mistake, but this is the sort of mistake we should try to handle gracefully.
Also remember our users want and expect VA to work and do something helpful as they are writing and editing code, so we are normally working on code "in progress", so code that does not actually compile, code that is not valid. If we were allowed to assume that all code was always valid it would make things easier, but that's not what we are trying to do. VA is not perfect, but we do work hard to make it useful and reliable. |
zen is the art of being at one with the two'ness |
|
|
|
legalize
Tomato Guru
USA
119 Posts |
Posted - Jun 17 2014 : 09:56:52 AM
|
Well, ok, if you want to consider selections that include portions of a token or malformed expressions, then yes, those don't make sense.
There is always a valid transformation when entire statements or proper expressions are selected. (By "proper expression", I mean any [expr] as specified by the standard.) |
http://legalizeadulthood.wordpress.com |
|
|
|
feline
Whole Tomato Software
United Kingdom
19024 Posts |
Posted - Jun 17 2014 : 1:55:06 PM
|
Some of our bug reports are actually due to invalid code confusing either VA or the IDE. This is simply a reality we need to consider and deal with. After all, if the code is not valid, does not match the standard before you trigger Extract Method, which is common while writing code, then we are going to see code we cannot understand. Plus we all make mistakes.
A very simple example, extracting references to an undeclared variable. Since the variable is undeclared, we don't know its type. Yet the whole point of the Create From Usage refactoring command is to help handle these invalid variables, and make them valid.
None of this is to say that more accurate handling of code in Extract Method, and other refactoring commands is not important, its just that its not the only thing we are working on and considering. |
zen is the art of being at one with the two'ness |
|
|
|
legalize
Tomato Guru
USA
119 Posts |
Posted - Jun 17 2014 : 2:21:28 PM
|
Create From Usage is a great tool, but it's not a refactoring tool. It's a code generating tool, like snippets. It may sound kind of nitpicky, but I really wish Whole Tomato would stop referring to these as refactorings because they aren't refactorings.
A refactoring transforms working code into another form that preserves the semantics.
I don't expect any refactoring tool to do useful things when starting with invalid code. |
http://legalizeadulthood.wordpress.com |
|
|
|
feline
Whole Tomato Software
United Kingdom
19024 Posts |
Posted - Jun 19 2014 : 4:06:36 PM
|
Given your definition I accept that Create From Usage is not a refactoring, but by this measure, a lot of other features that exist and that in general are grouped under this heading are not refactorings either. So unfortunately this is a niggle I think you are going to have to get used to, since its not just us.
As for starting with invalid code, a lot of our users expect VA to work correctly, within certain limits, with invalid code, and get upset when it doesn't. This is the simple reality of helping while you are working on code. |
zen is the art of being at one with the two'ness |
|
|
|
legalize
Tomato Guru
USA
119 Posts |
Posted - Jun 22 2014 : 1:09:29 PM
|
No, really, it is just you. Noone else in the industry refers to code generation as refactoring. Refactoring has a very specific meaning and usage throughout the industry and it's Whole Tomato that's out of step here. The Gold Standard for refactoring is set by JetBrains with their ReSharper and IntelliJ products. JetBrains tools also have tons of quick fix helpers, code snippets and other features that make it very helpful when writing code. These kinds of tools are very useful, but they aren't refactoring.
It is extremely disappointing to me that a refactoring tool vendor doesn't understand the meaning of the word refactoring and insists on applying that name to tools, that while useful, are not refactorings. This isn't just a matter of "getting used to it", it generates confusion and mislabels your features. I don't know why you would think that is a good thing. |
http://legalizeadulthood.wordpress.com |
|
|
| |
Topic |
|
Extract Method creates invalid code
