|T O P I C R E V I E W
||Posted - Jun 13 2014 : 12:16:03 PM
0% Extract Function: 0 passed, 18 failed
45% Extract Method: 9 passed, 11 failed
63.55% Rename: 61 passed, 35 failed
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
= Extract Method
= Move Method to Header/Source
|9 L A T E S T R E P L I E S (Newest First)
||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.
||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.
||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.
||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.
||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:
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.
||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.
||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.
||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.
||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.