| T O P I C R E V I E W |
| legalize |
Posted - Jun 13 2014 : 12:16:03 PM
Summary:
0% Extract Function: 0 passed, 18 failed
45% Extract Method: 9 passed, 11 failed
63.55% Rename: 61 passed, 35 failed
Test suite:
http://legalizeadulthood.wordpress.com/2010/02/02/c-refactoring-tools-test-suite-available/
Details:
VA_X.dll file version 10.8.2036.0 built 2014.05.22
All tests were performed using suggested selections in dialogs.
= Change Signature
= Create Declaration From Implementation
= Create Implementation From Declaration
= Create Implementation From Usage
= Extract Function
EXF1 Fail
EXF2 Fail
EXF3 Fail
EXF4 Fail
EXF5 Fail
EXF6 Fail
EXF7 Fail
EXF8 Fail
EXF9 Fail
EXF10 Fail
EXF11 Fail
EXF12 Fail
EXF13 Fail
EXF14 Fail
EXF15 Fail
EXF16 Fail
EXF17 Fail
EXF18 Fail
= Extract Method
EM1 Pass
EM2 Fail
EM3 Fail
EM4 Pass
EM5 Fail
EM6 Pass
EM7 Pass
EM8 Fail
EM9 Fail
EM10 Pass
EM11 Fail
EM12 Pass
EM13 Pass
EM14 Fail
EM15 Fail
EM16 Pass
EM17 Fail
EM18 Pass
EM19 Fail
EM20 Fail
= Move Method to Header/Source
= Rename
R1 Fail
R2 Fail
R3 Fail
R4 Fail
R5 Pass
R6 Pass
R7 Fail
R8 Fail
R9 Pass
R10 Fail
R11 Pass
R12 Fail
R13 Pass
R14 Fail
R15 Fail
R16 Pass
R17 Fail
R18 Pass
R19 Fail
R20 Fail
R21 Fail
R22 Fail
R23 Fail
R24 Pass
R25 Pass
R26 Fail
R27 Fail
R28 Fail
R29 Fail
R30 Fail
R31 Pass
R32 Fail
R33 Fail
R34 Pass
R35 Pass
R36 Fail
R37 Pass
R38 Pass
R39 Pass
R40 Pass
R41 Fail
R42 Pass
R43 Pass
R44 Pass
R45 Fail
R46 Pass
R47 Pass
R48 Pass
R49 Pass
R50 Pass
R51 Pass
R52 Pass
R53 Pass
R54 Pass
R55 Pass
R56 Pass
R57 Pass
R58 Pass
R59 Pass
R60 Pass
R61 Pass
R62 Pass
R63 Pass
R64 Pass
R65 Pass
R66 Pass
R67 Pass
R68 Pass
R69 Pass
R70 Pass
R71 Pass
R72 Pass
R73 Pass
R74 Fail
R75 Fail
R76 Fail
R77 Fail
R78 Fail
R79 Fail
R80 Fail
R81 Fail
R82 Pass
R83 Pass
R84 Pass
R85 Pass
R86 Pass
R87 Pass
R88 Pass
R89 Pass
R90 Pass
R91 Pass
R93 Pass
R95 Pass
R96 Pass
R97 Pass
R98 Fail
|
| 9 L A T E S T R E P L I E S (Newest First) |
| feline |
Posted - Jul 03 2014 : 10:39:24 PM
VA is broadly the same in all IDE's, but each IDE is different, and this can have an effect. It depends on what you are doing. Refactoring commands should work the same in all IDE's nearly all of the time. The main difference you will find is that VS2013 does its own code formatting, possibly changing any code formatting you have set VA to apply. Plus some commands are not available in older IDE's, this mainly effects VC6.
The state and editing history can have an effect, but should not effect refactoring, but it will effect suggestion listboxes.
Occasionally bugs are IDE and OS specific, its rare but it does happen.
Solution differences seem the most likely reason you will get different results in VS2012 and VS2013. |
| legalize |
Posted - Jun 22 2014 : 2:44:50 PM
Are the implementations of VAX different for VS2012 and VS2013?
My initial test results were created in VS2012 and I just retested the failure cases in VS2013 in order to create specific bug reports for them and some of my failed cases now pass. I'm using the same build of VAX in both cases.
I suppose it's possible that only repeating the failed test cases didn't manage to get VAX into the same state as running all the test cases end to end. |
| feline |
Posted - Jun 14 2014 : 12:17:03 AM
I have a vague memory of you posting this once before, apologies for not getting to them sooner. We get a constant stream of bug reports, feature request and other enquiries. Against this background, your test solution is not an easy bug report to process. I have been looking at this for a few hours now and I am still working through the first file I started in, "ExtractFunction.cpp".
If all of the files are this slow going, this is going to take weeks, since I cannot give it my full attention.
I agree that placing free functions into a namespace is sensible and reasonable. Based on bug reports some bugs are simply more likely to be discovered than others, so those are the ones we prioritise, and until this one has never come up. Perhaps because free functions don't come up in most bug reports. |
| legalize |
Posted - Jun 13 2014 : 6:56:24 PM
I posted the test suite 4 years ago.... http://forums.wholetomato.com/forum/topic.asp?TOPIC_ID=8732
Those tests have been in there all that time.
Having free functions inside a namespace is really common because namespaces are how you keep those functions segregated from polluting the global namespace. |
| feline |
Posted - Jun 13 2014 : 5:31:43 PM
A lot of my testing of Extract Method over the years has been done on free functions, since it removes the added variable of the class, and until now I have never seen this particular bug. After a bit of testing I know why, the ordering problem only occurs when free functions are placed inside a namespace. Remove the namespace and the ordering is correct, the new function is placed above the original function. I have put in a bug report for this:
case=82954
Now before you say this is an obvious bug we should have caught earlier  I don't think anyone has ever reported this before. Certainly I am not finding any records of such a bug report.
We work hard to make all features of VA work correctly all of the time, but there are bugs. Its my understanding that all complex programs have bugs. So we have made the pragmatic decision to settle for works well enough most of the time for most people, and keep on fixing the bugs as they come up. |
| legalize |
Posted - Jun 13 2014 : 4:44:02 PM
Yes, in the VAX UI it's called Extract Method in both places. But if it is only supposed to work for classes and not for functions, then it should be disabled when I'm in a free function context. Either way, you can look at it as a bug.
If it's available, then it should function correctly.
If it can't function correctly in that context, then it shouldn't be available. |
| feline |
Posted - Jun 13 2014 : 4:19:46 PM
This makes sense. I have just started looking at "ExtractFunction.cpp" and noticed the ordering problem. I don't recognise this off the top of my head, so I am not sure anyone has ever reported this before. So I want to run some tests and see why.
As you will have noticed, VA uses one refactoring, Extract Method, for both free functions and class member functions, we don't make a distinction here.
In the same file, test EXF1 is a known limitation, currently VA expects and requires you to select all uses of the variable if you have selected the declaration. This is something we are looking into, but we cannot stop you asking VA to perform an extract method that does not make much sense, all we can do is try to notice this and warn you. |
| legalize |
Posted - Jun 13 2014 : 4:06:20 PM
If I'm in a class and I extract some selected code into a method, then I'm applying Extract Method.
However, unlike Java, C++ doesn't require everything to be in a class. I can certainly have free functions from which I extract other functions.
The main reason that all the Extract Function test cases fail is because VAX *always* puts the extracted code *after* the source from which it was extracted. This means that the original code references an undefined/undeclared function when you compile it.
There are a couple other places where it was the same bug affecting different test cases. If I have the time, I'll go back and file specific bug reports for those specific cases.
Off the top of my head:
- pointers to member functions and pointers to functions confuse the signature of the extracted function and result in syntax errors. When the value was referenced by a typedef instead of the inline syntax of function pointers or pointers to methods, then it worked OK.
- Data flow analysis is weak. Sometimes extract moved declarations from the parent scope to the scope of the extracted function, making all other references to the declared item invalid.
- Sometimes the functionality was enabled on the menu but didn't do anything
I've updated the page for the test suite on my blog with the explanation from the download location so it's a little more obvious how this test suite works. |
| feline |
Posted - Jun 13 2014 : 4:00:39 PM
Interesting, I have downloaded this, and am starting to look now. It may be a while before I have much sensible to say about this, since it looks like quite a lot of tests to understand and wrap my head around.
I am immediately struck by the fact that you list "Extract Function" separately to "Extract Method", and am curious as to what the difference is. Normally I would say a function and a method are the same thing. VB has functions and sub's, but this is a C++ project, so that's not the answer. |
|
|
