| T O P I C R E V I E W |
| Frunobulax |
Posted - Feb 05 2008 : 06:28:34 AM
Hi,
currently C++ create implementation always places the new body at the end of the implementations of the class (AFAIK). Wouldn't it make more sense to place it between the implemtations of the surrounding functions, if available? Usually functions are grouped by some criteria, both in the header and in the implementation. If I add a function signature to a group then I want the implementation to be in the implementation group.
Crucial are cases where partial functionalities are controlled by #define - often I have a block
#ifdef SOME_FEATURE
...
#endif
in both headers and source, and in this case it is essential whether a new implementation is inside or outside of the #ifdef block. Keeping to the surrounding functions fix that.
Regards, Thomas
|
| 24 L A T E S T R E P L I E S (Newest First) |
| Frunobulax |
Posted - Mar 14 2008 : 07:53:05 AM
quote:
Frunobulax I am wary of this, since any complex rule is hard to explain. When people start asking "why is VA doing X" how do I answer them?
Magic 
A quick description would be "try to locate the implementations of the surrounding functions. If they are consecutive, place the new function between them. If they aren't, guess."
quote:
Looking at the clauses I can immediately see two "problems". Firstly VA cannot tell which overload is which.
Again, if the overloads follow each other you can just use the first or last. If they don't follow each other - too bad for the user.
quote:
Secondly in point 3 you mention the case where the function order is reversed in the cpp file. But what happens when they are in the wrong order and there are 30 functions in between them?
No, that is the case where the functions are in order and consecutive. Point 4 explains what happens if they aren't. Sorry if I didn't make that clear.
quote:
Did you notice:
>> case=12454, which is referenced in this thread, has the title:
>> Create Implementation to place new code below the implementation of preceding function
This is only a simple rule, but it seems like a reasonable idea on paper.
This would work fine for me. But some people in this thread seemed to think that they need something more sophisticated...
Regards, Thomas
|
| feline |
Posted - Mar 13 2008 : 2:49:42 PM
hajokirchhoff I am not sure how well this would work. Ignoring the problem of several classes in the same file (which really complicates this idea) you have to keep moving the comment around the cpp file as you work. I don't see the advantage here. VA Outline makes it quick and easy to move the newly added function, so you might as well just move that.
Frunobulax I am wary of this, since any complex rule is hard to explain. When people start asking "why is VA doing X" how do I answer them?
Looking at the clauses I can immediately see two "problems". Firstly VA cannot tell which overload is which. We do hope to fix this, but until it is fixed this is going to cause problems for any rule more complex than "put the new function at the end"
Secondly in point 3 you mention the case where the function order is reversed in the cpp file. But what happens when they are in the wrong order and there are 30 functions in between them?
Did you notice:
>> case=12454, which is referenced in this thread, has the title:
>> Create Implementation to place new code below the implementation of preceding function
This is only a simple rule, but it seems like a reasonable idea on paper. |
| Frunobulax |
Posted - Mar 13 2008 : 12:15:16 PM
quote:
Originally posted by feline
In your example this is the "wrong" position. However I don't think there is a "right" position in your example.
[...]
This is the reason for the current behaviour, placing the function at the "end". It may be wrong, but at least it is simple to understand.
Lots of people want VA to do something more sensible, but no one seems to know what that more sensible thing is. case=12454 is a compromise.
[...]
I am open to good suggestions for a general solution 
Sorry for replying late - I was travelling for some time...
Yes, it is hard to find a "sensible" behaviour. I will try to suggest something that makes sense for most programmers.
So what are the options?
- Place implementation always at the end (as it is now).
- Use a simple rule to place the implementation.
- Make a complex analysis about the file is structure and guess the position.
Option (1) makes sense if you like to keep new functions at a defined position of the source file - go to the end if you want to be where you are currently working.
However, I would assume that most programmers group their implementations in some ways and don't like the "place implementation at the end" option. Then either the grouping is straightforward, in which case a simple rule (option 2) will place the function at the right spot in most cases, or it is complicated, in which case even a complex analysis will probably fail. So I'd say option (3) makes no sense and we stick to a simple rule, possibly with a configuration option deciding between configurable and "end of file".
How could a simple rule look like? Here is what I would do for a new function F:
- Parse the preceding functions and find the last function A with known implementation place preceding F.
- Parse the succeeding functions and find the next function B with known implementation following F.
- If both A and B are not found, place the implemtation of F at the end of the source file.
- If only one of A and B was found, place F after the implementation of A or before the implementation of B.
- If both A and B were found and B follows A in the implementation file, insert F between A and B.
- If both A and B were found and B does not follow A in the source file and dist(A,F) != dist(F,B), place F after A or before B, whatever is closer. Here dist(X,Y) is the number of statements (non-comments) between the declarations of X and Y.
If dist(A,F) == dist(F,B), place F after A. This will still be correct in at least 50% of the cases :-)
There may be cases where VAX is not sure where the implementation of a function is (like your overload case). A simple way would be taking a guess (use the last of the candidates when looking for A, or the first of the candidates when looking for B), which should be good enough in most cases.
I'm pretty sure that this will find the correct position in 90% of all cases, which would be a 90% improvement over the current situation for me :-)
Regards, tv
|
| hajokirchhoff |
Posted - Mar 13 2008 : 03:55:39 AM
Regarding the positioning of implementation/declaration code generated by VAssist:
90% of the time I am satisfied with where VA inserts new code. For the remaining 10% where the default behaviour does not suit my needs I would want to insert a special comment into the header/cpp file such as
///@VA-insert-point
or something like this. When VA finds this comment inside a class declaration, it will insert new methods below this comment. Same/similar comments could tell VA where to insert code in the .cpp files. This would work similar for other languages. And for the very neat programmers it should be possible to have more than one such comment inside a class, for different visibilities public, private etc...
Best regards and thanks for making my life easier ;)
Hajo |
| feline |
Posted - Mar 07 2008 : 3:15:22 PM
znakeeye if you are seeing this in the current version of VA can you please start a new thread for that problem?
I recall a couple of reports of this, but it only happened under Very specific conditions. |
| znakeeye |
Posted - Mar 07 2008 : 03:10:21 AM
I hope you don't forget about the issue where the implementation is placed inside the BEGIN_MESSAGE_MAP macro. Quite frustrating bug :( |
| feline |
Posted - Mar 03 2008 : 09:53:21 AM
From the support point of view, if someone sorts a file, VA gets it wrong, and the code stops compiling, this is a big problem.
With rename, which can also be "dangerous" VA shows you what it is going to do, so you can review it before asking VA to make the code changes. |
| KoenTanghe |
Posted - Feb 29 2008 : 6:39:17 PM
@feline: sure I read the thread, I just wanted to say that I too find both points useful, and that I wouldn't mind it to work only in 90% of the cases. If it didn't work well in the other 10%, we can still use VA outline then. |
| feline |
Posted - Feb 29 2008 : 08:39:16 AM
Have you read this thread? Both points are discussed here |
| KoenTanghe |
Posted - Feb 28 2008 : 08:19:40 AM
VA Outline is great indeed.
But I also would love to have these 2 features:
- one that creates the implementation in the .cpp file right after the implementation of the preceding member function from the header (given a certain member function declaration in the header)
- one that synchronizes the order of the member function declarations in the header with the order of their implementation in the .cpp file; this could work both ways: in .cpp file "sort using order from .h file", or in .h file "sort using order from .cpp file", but I'd be glad if the first case would exist: my header file is usually the master
I don't care if it doesn't work in some special cases, if it works in 90% of the cases that'd be already great!
|
| baccardi |
Posted - Feb 22 2008 : 2:40:47 PM
Ah yes, I can see how that can be a problem...
Fortunately, that outline feature is excellent and will satisfy my needs!
Thanks for the heads up! |
| feline |
Posted - Feb 22 2008 : 09:03:56 AM
While the idea is appealing, I suspect it would run into problems. What happens with comment's that are not related to any function? #include lines in the middle of a cpp file? using namespace foo; lines at odd places in a cpp file?
I have seen all of these, and done some of them myself, for good reasons at the time.
VA Outline is designed to allow you to quickly and easily re-order the current file yourself. Since you are re-ordering the file manually you can watch for, and handle, any oddities in the file. |
| baccardi |
Posted - Feb 21 2008 : 10:14:45 PM
One refactoring feature that I have wanted for a while is re-arranging the cpp file based on the order of the h file. I find when I am doing a big refactor, I tend to start by logically arranging my header file first, and then moving my cpp file contents around to reflect what is in the h file.
If there was a feature like this than it could be a two step feature similar to 'Extract method' -> 'Move Implementation to Source File' process. It would be something around the lines of 'Create Implementation' -> 'Rearrange Source file to Match Header File Order'.
It seems like this would satisfy a majority of these issue, maybe throw an alphabetical sort in there for those that want their functions in alphabetical order. |
| feline |
Posted - Feb 19 2008 : 1:03:17 PM
There are a couple of problems with deciding if the two files are in the same order.
First problem, VA is currently "confused" by overloaded functions. It does not know which overload is which. We are hoping to fix this with time.
Second problem, as soon as you add more than one new declaration to the header file, or have inline functions in the header file, there are going to be "missing" files in the cpp file. This is not a total problem, but it is definitely possible to imagine cases where it gets quite complex, if you have inline functions implemented inside the class in the header file and also after the class declaration, still inside the header file.
I would like to see a complete fix for this myself. Unfortunately the more I think about the problem, the harder it gets to solve.
If some for of "best guess", based on a single function above or below was done, and it was optional, people could turn it on if they found it worked well for them. I agree it is likely to cause problems. I am not even sure it is a really good idea, but it will probably work fairly well quite a lot of the time. That might be good enough. |
| accord |
Posted - Feb 19 2008 : 12:08:08 PM
"is there any good general solution?"
No, there is not.
My opinion is that if surrounding functions in the header are cannot be found in the cpp in the same order, then fall back to the "end of the functions" method rather than into a half-sensible one, which will produce totally not sensible solutions sometimes.
So my solution is all or nothing (perfect or predictable result).
Half sensible solutions may seem random or buggy at edge cases. |
| feline |
Posted - Feb 19 2008 : 09:01:59 AM
I think I see now.
As soon as the header file and cpp file are in different orders, as they are in your example, is there any good general solution? Looking at the function declaration above in the header file instead of the function below probably does not make a lot of difference in general. Both approaches are going to be wrong some of the time.
It should be very easy to think of a test case where the function above produces the "wrong" result. Just as it is easy to produce a test case where the function below produces the "wrong" result. |
| accord |
Posted - Feb 18 2008 : 3:30:50 PM
Sorry, I am quoted you from another thead only because of this sentence: "Worker functions before the functions that use them."
I am doing this often. So "Create Implementation to place new code below the implementation of preceding function" will fail in these cases. But I see now: you do think that the ordering will be more likely very different than similar, so surroundings will NOT be enough.
This is the main difference in your and my opinion if I see correctly.
Your solution: below implementation or end
My solution: surrounding functions or end
So I do not want to make something optional 
Only wanted to so you an edge case. I think this edge case can be eliminated by verifing surrounding functions. But you think it won't help because the very different ordering of the header and the cpp.
Am I right? |
| feline |
Posted - Feb 18 2008 : 11:31:20 AM
That looks like my comment from the thread about ordering the functions into alphabetical order. I am not sure that it helps here.
case=12454, which is referenced in this thread, has the title:
Create Implementation to place new code below the implementation of preceding function
In your example this is the "wrong" position. However I don't think there is a "right" position in your example. Place the function at the end, and people complain that the ordering of the header file is being ignored. Use any other simple rule, and you are going to get it wrong some of the time, since the two files seem to have no relationship to each other, or at least no ordering that is obvious to me.
This is the reason for the current behaviour, placing the function at the "end". It may be wrong, but at least it is simple to understand.
Lots of people want VA to do something more sensible, but no one seems to know what that more sensible thing is. case=12454 is a compromise. I am just not sure how good a compromise. I have just put a note on the case, saying it probably needs to be made optional. However there is no guarantee we will actually do this.
I am open to good suggestions for a general solution |
| accord |
Posted - Feb 16 2008 : 4:31:51 PM
Let me show you an example:
cpp:
void func6();
void func2();
void called_by_func5(); // <-- I use create declaration on this function
void func5();
void func3();
void func4();
void func1();
void func7();
quote:
feline said:
Personally I often have my functions in a sensible order, but that is nothing like alphabetical. Private functions tend to come first. Worker functions before the functions that use them. Functions for a common purpose tend to be grouped together as well.
So I do the same what you usually do. But if VAX will check only the previous function, VAX will place called_by_func5() into the wrong place in the header, after func2():
header:
void func6();
// this is where VAX should place the new function (before func5())
void func5();
void func3();
void func4();
void func1();
void func2();
void called_by_func5(); // this is where VAX will place the new function (after func2())
void func7();
This will happen because VAX don't know about how the functions grouped together.
This is why I said that if surrounding functions are different it is better to place new function into the end of the file rather than into an invalid position. Users may think this is a random behavior or a bug.
What do you think? |
| feline |
Posted - Feb 11 2008 : 10:27:31 AM
Using the surrounding functions assumes that the header and cpp files are in the same order. It is easy to imagine, and actually find, function in the order A, B, C in the header file and the order C, B, A in the cpp file. In this case you cannot place the function in the correct position.
Using a much more "limited" form of correct positioning is going to do something half sensible some of the time, even when the files are in quite different orders. |
| accord |
Posted - Feb 10 2008 : 11:30:09 AM
I think VAX should allways try to place the implementation between the surrounding functions, if available.
Watching only the preceding function will not be enough, because sometimes I am placing new functions *before* the original one. So if the surrounding functions are not in the same order in the header and in the cpp, VAX should allways fall to the default (currently active) method.
What do you think?
This is because VAX cannot figure out that I want to group my new function with the preceding or with the next function. |
| feline |
Posted - Feb 07 2008 : 10:22:52 AM
I can see some edge cases where this will run into problems, but I like this idea. It should help quite a lot of the time. I have put in a feature request to see what our developers make of it:
case=12454 |
| Frunobulax |
Posted - Feb 07 2008 : 04:26:45 AM
quote:
Originally posted by feline
The basic problem is that normally the header file and cpp files have the functions in a different order. As soon as this starts happening VA has no easy way of working out the correct position.
Still, it would be possible to place the code below the implementation of the preceding function. Maybe the functions are in different order *because* VAX always places the implementation at the end of the file? ;-)
BTW, I would guess the change of behaviour I suggested would be a rather trivial change. If VAX can't determine where to place the functions it could always fall back to placing the function bodies at the end.
quote:
For now have you looked at VA Outline? It is designed to allow you to drag functions, re-ordering the file quickly and easily.
The real work is to determine where the function should go, and not the actual moving.
Regards, tv
|
| feline |
Posted - Feb 05 2008 : 1:45:45 PM
The basic problem is that normally the header file and cpp files have the functions in a different order. As soon as this starts happening VA has no easy way of working out the correct position.
Placing the new code into the same #if or #ifdef block is a special case that we are considering doing something about:
case=9020
For now have you looked at VA Outline? It is designed to allow you to drag functions, re-ordering the file quickly and easily. |
|
|
