| Author |
 Topic  |
|
|
legalize
Tomato Guru
USA
119 Posts |
 Posted - Mar 25 2013 : 11:26:03 AM
|
The Change Signature refactoring tries to allow me to change anything in the signature by literally rewriting it. Every time I have used this refactoring, it warns me that it couldn't update all the call sites and I should revisit them manually.
Well, sorry, but if you couldn't do it automatically in a reliable way, then you shouldn't be offering this as a refactoring. I might as well just do regexp based search/replace myself.
I think the root problem of this is that Change Signature is trying to do too much. There are several common operations I often want to do when changing the signature of a function:
- Add or remove it's const qualifier
- Reorder the parameters
- Change the return type of the method
- Change an 'out' parameter to be the return type and eliminate the 'out' parameter (this is often the result of Extract Method confusing the result of a chunk of code with a modifiable parameter)
I think if these operations were provided more explicitly as individual refactorings, then they would have more of a chance of success instead of just dumping the whole signature into a text box and letting me free-form edit the signature. |
http://legalizeadulthood.wordpress.com |
|
|
accord
Whole Tomato Software
United Kingdom
3287 Posts |
Posted - Mar 25 2013 : 3:39:13 PM
|
| The basic idea here is that you can edit the signature both at the declaration and the implementation, since C++ makes you write them twice. One bonus is that VA catches renames and trigger a rename for the references in the method body. But VA don't try to change the calls (or the methods in the derived classes) hence the warning. |
 |
|
|
feline
Whole Tomato Software
United Kingdom
19022 Posts |
Posted - Mar 25 2013 : 3:39:27 PM
|
Change Signature is designed to help, but it depends on how much help you want VA to offer. Taking an "easy" option, making a function const is fine, but it then requires analysis of everything that function does, all the way down, to see if it is actually const. The compiler does this when you compile, and it is something we are considering for VA, at least to a limited depth, but its not always an easy thing to check.
Changing the return type of the method requires rewriting the method to some degree or other. Even if you said to VA "use this local variable as the return type", VA would not know how to update all of the function calls, or how to fix the functions internal logic.
The current free editor for Change Signature is designed to be quick and easy. Otherwise you might end up with a much more complex dialog, asking about several different changes, or a set of refactoring commands that all run into the same problem as Change Signature, updating the function declaration is easy, but the function calls and implementation need human intervention.
Or do I misunderstand what you are after here? |
zen is the art of being at one with the two'ness |
|
|
|
legalize
Tomato Guru
USA
119 Posts |
Posted - Mar 25 2013 : 4:43:59 PM
|
If I understand accord correctly, VAX never, ever tried to fix the call sites, so to me this refactoring is of extremely limited usefulness.
I get the distinct impression that VAX developers are making "refactorings" in their own little universe without ever having used any competing products that have extensive refactoring support. DevExpress CodeRush has the ability to do refactorings like reorder parameters, change the signature by promoting arguments to return types, etc. While I love the large number of refactorings that they have, the last time I evaluated their product several years ago, it just made the whole IDE way too slow, which is why I've been using and recommending VAX.
For DevExpress, I think the main reason their stuff is so slow is because they wrote it all in .NET, whereas VAX seems fast exactly because it's written in C++ with an eye towards efficiency. I want to have my cake and eat it too: I want the advanced refactoring support from the CodeRush product with the execution and memory efficiency of VAX.
For a comparison outside the C++ world, take a look at IntelliJ IDEA's Java refactoring support. Yes, they have some advantages in parsing Java compared to the difficulty of parsing C++, but the refactorings available there are extremely useful and it would be a huge win to have them in C++ with VAX.
Now that I understand what Change Signature was designed to do (just synchronize the same edit to the .h and .cpp), it's limited usefulness is obvious. However, I think that "Change Signature" is a really poor name for this feature because in other refactoring tools, Change Signature really does propagate the change through to all the call sites. It may leave you with invalid code at the call sites and that's OK because I can "Lean on the Compiler" to get those changes propagated out.
However, imagine I have a function that takes two std::string parameters and I use Change Signature to swap them. Swapping the parameters at all the call sites of the function is an entirely reasonable expectation and something that's also entirely within the reach of VAX to do for me.
That's why I was saying that this refactoring sounds like it's trying to do too much (now I realize it wasn't trying at all). The wording of the warning always left me with the impression that VAX tried to fix the call sites and simply failed. Maybe I just didn't read the warning closely enough because I'm used to Change Signature in other refactoring tools actually propagating the change through to the call sites. |
http://legalizeadulthood.wordpress.com |
|
|
|
accord
Whole Tomato Software
United Kingdom
3287 Posts |
Posted - Mar 27 2013 : 12:41:17 PM
|
I have put in a feature request for this to see what the developers make of it:
case=73302
Creating more, separate refactoring commands may help with updating call sites. |
Edited by - accord on Mar 27 2013 12:42:41 PM |
|
|
|
legalize
Tomato Guru
USA
119 Posts |
Posted - Mar 27 2013 : 1:22:51 PM
|
| I think Reorder Parameters is one where the refactoring could be completely automated and the call sites can be updated without difficulty. That one alone would be very useful. |
http://legalizeadulthood.wordpress.com |
|
|
|
feline
Whole Tomato Software
United Kingdom
19022 Posts |
Posted - Mar 27 2013 : 3:17:23 PM
|
Reorder Parameter is a good example of a separate command, and I agree, it should be fairly easy to do. Making the existing refactoring commands, limited as they are, properly reliable can be surprisingly hard, there are a lot of edge cases out there. Just because it is hard is no reason not to try though, and you have a very good perspective on this, thank you for the ideas, and please keep them coming  |
zen is the art of being at one with the two'ness |
|
|
|
sean
Whole Tomato Software
USA
2817 Posts |
Posted - Nov 18 2013 : 3:22:23 PM
|
Change Signature was overhauled in build 2007.
References are updated when parameters are added, removed or reordered. |
|
|
| |
Topic |
|
Change Signature tries to do too much
