| Author |
 Topic  |
|
|
AdrianH
Ketchup Master
Canada
71 Posts |
 Posted - Oct 11 2013 : 4:49:11 PM
|
Your code extraction tool is really buggy. I.e. It doesn't work on my code. It misses quite a few variables and does other stuff that's really bad (dealing with return statements, unnecessary parameter passing, parameter order is randomized).
Is there any way for me to determine if a symbol is a local variable, a parameter or something else (when I say that, I really mean I don't care what it is if it's not a local var or parameter). With just that info, I think I can use the MSVS IDE's API to write a better extractor.
I have the algorithm to generate the code extraction tool, but I would rather not reinvent the wheel by creating a new parser (though if necessary, I could probably hack one together).
Alternatively, I could give you the algorithm, but I'd like it implemented ASAP, as I need this available ASAP.
Thanks,
A |
Edited by - AdrianH on Oct 11 2013 5:15:30 PM |
|
|
accord
Whole Tomato Software
United Kingdom
3287 Posts |
Posted - Oct 12 2013 : 06:43:24 AM
|
| It would be helpful if you could post some code samples code where extract method fails, so we could look into the problems. |
 |
|
|
AdrianH
Ketchup Master
Canada
71 Posts |
Posted - Oct 15 2013 : 09:47:10 AM
|
Sorry, the code is to complex and I'm not authorized by my work to post it.
Oh, and an FYI, the reason that I really need this to work and work flawlessly is because I've got to break apart a couple of functions that are 1000-1500 lines of code. |
Edited by - AdrianH on Oct 15 2013 2:31:26 PM |
|
|
|
feline
Whole Tomato Software
United Kingdom
19025 Posts |
Posted - Oct 15 2013 : 10:14:52 PM
|
Extract Method should be more reliable than this, apologies for the problems.
When you say Extract Method is missing a variable, what do you mean? Are you ending up with an undeclared variable variable in the new function that is not passed in? Or do you mean something else?
One known limitation with Extract Method is that if you select and extract the declaration of a local variable, VA expects you to have selected all instances of the variable, so if this is happening this would explain some, but not all of your problems.
What problems are you seeing with return statements? Could you post just a couple of lines of code to try and explain what you you are seeing? |
zen is the art of being at one with the two'ness |
|
|
|
AdrianH
Ketchup Master
Canada
71 Posts |
Posted - Oct 16 2013 : 09:46:28 AM
|
Yes, undeclared variables. Also the unnecessarily passing of variables that are either not used or are reinitialised after the selection being extracted (SBE).
Here is my requirements/wish list:
The parameter list generated for the SBE should be (IMO) a list of variables that are (in this order):
- used within the SBE that are from the original parameter list, in the same order as they appear in the parameter list
- used within the SBE as they are declared within the function, in the same order as they are declared in the function
Variables that are declared in the original function that are not used after the SBE and are not operated on before the SBE should be just pulled right out of the original function and moved to the new one.
Variables that are declared in the original function that are reinitialised in all code pathways prior to being operated on after the SBE and are not operated on before the SBE should also be pulled out of the original function and copied to the new one (or an option to do so should be presented) while optionally moving the original declaration after the SBE to the same scope level.
Should also work with SBE's containing lambdas, and probably be a good idea that it works within lambdas too (though that is low priority).
SBE's containing returns have to look at all execution pathways to determine if all pathways contain a return. If they do then the replaced code can just be a return of the new SBE's function. Otherwise it should do something like return an std::optional and check if it has been initialised, returning if it has, continuing if it hasn't.
This is probably an incomplete list, but should get you started.
If VA expects that the SBE contains the all uses of a declared variable, then that's a problem too, but not one I experienced. The extraction I did was the body of a for loop, so anything declared would not have scope outside of it anyway.
Do you have a maximum number of parameters that you will generate? That could have been part of the reason for the variables not all being passed/declared, though I've not verified.
I'm soooooo busy right now. I'll see if I can get you some more manageable examples in the coming weeks. |
|
|
|
feline
Whole Tomato Software
United Kingdom
19025 Posts |
Posted - Oct 16 2013 : 11:18:11 PM
|
Passing the parameters first, and then the local variables, all in order makes sense. I have put in a feature request for this:
case=77703
Currently, as you have probably noticed, the parameters are passed in the order they are used in the extracted code.
We are looking to make Extract Method extract the variable declaration when you have selected all uses of a local variable:
case=63055
The idea of picking out when a local variable being reused has come up before, but making this part of Extract Method makes sense:
case=77697
Moving the declaration of a local variable to narrow its scope is something we are considering adding as a refactoring command:
case=30486
Are you working with Lambda's in your code? I don't really know anything about these yet, but support was added to VA quite a while ago.
We are considering checking for conditional return statements in the extracted function, so I have put a note onto the case about knowing if we should return or continue:
case=4126
The number of parameters, Extract Method is topping out at 20 parameters in my testing. Are you finding that you need more than 20 parameters? I hope not, but its certainly possible. |
zen is the art of being at one with the two'ness |
|
|
|
AdrianH
Ketchup Master
Canada
71 Posts |
Posted - Oct 17 2013 : 07:57:59 AM
|
This code is at least 20 years of legacy. Some of the functions start off with 10 parameters and I've seen as many as 34  . This code really needs refactoring, that's why I can't have parameters passed if they really should be local.
Also, promoting of variables to be parameters or class member variables would be nice too. Possibly along with demoting (for a member var, it would have to be used in only one function).
Parameter rearrangement with auto updating of calling functions would be nice. I know that you have a way of changing the signature, but it doesn't (and wouldn't) know how to update callers. However, if a signature is changed, it would be nice to get a list of callers that one can step through (like using the F4 command when doing a find in files or when iterating through compiler errors). Dependency checks between modified vars used in the parameters isn't an issue as the order of evaluation is not determined in the C++ spec. |
Edited by - AdrianH on Oct 17 2013 08:38:37 AM |
|
|
|
feline
Whole Tomato Software
United Kingdom
19025 Posts |
Posted - Oct 18 2013 : 2:56:41 PM
|
If this was pure C code then this many parameters would make a bit more sense, but even then, this is really nasty and hard to work with. You have my sympathies here, for what its worth.
A random thought, for some of the worst cases, have you considered making a struct and passing the parameters inside the struct instead? Perhaps too much overhead to be worth it, but it might help on occasion.
We are working to make sure Extract Method does not pass to many parameters, and your comments here will help with that, thank you. Are you having situations where Extract Method is actually having to pass more than 20 parameters to the new functions, so you are encountering this limit?
We are considering adding a command to promote a local variable to a parameter or class member:
case=43947
Demoting a member variable to a local variable makes sense, and its a good idea, I have put in a feature request for this:
case=77760
We are working to make Change Signature smarter, and updating the calling functions when the parameters have been reordered is one of the things we want to do:
case=73302
When Change Signature detects a change in the number of parameters it should show a message box telling you this, and automatically perform a Find References on the function you have just updated. At this point pressing F4 will move you through the Find References Results one at a time, exactly as you are looking for.
A similar message is appearing when I reorder parameters in a function.
Are you ever seeing any sign of these message boxes?
It turns out there is a registry key to suppress the message boxes, but even if this key is set the Find References is still run after the Change Signature, at least for me.
Can you check your registry please and see if you can find a value called "NagAfterChangeSignature" under the:
HKEY_CURRENT_USER\\Software\\Whole Tomato\\Visual Assist X\
key? If present it should be in the IDE specific sub key, but I am not sure which IDE you are using. |
zen is the art of being at one with the two'ness |
|
|
|
AdrianH
Ketchup Master
Canada
71 Posts |
Posted - Oct 18 2013 : 3:23:15 PM
|
I can't verify these anymore as my trial ran out and given the state of VA X, my manager won't go for getting a licence. Looks like I'll have to find some other means.
Thanks,
A |
|
|
|
feline
Whole Tomato Software
United Kingdom
19025 Posts |
Posted - Oct 18 2013 : 11:54:35 PM
|
| I have emailed you about this. |
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:50 PM
|
Change Signature was overhauled in build 2007.
References are now updated when parameters are reordered. |
|
|
| |
Topic |
|
API request
