| Author |
 Topic  |
|
|
Luke1410
Senior Member
32 Posts |
 Posted - Jan 18 2011 : 03:53:39 AM
|
Minor suggestion, but I just ran into that case, so I'd at least like to mention/suggest it here. :)
sample code:
class Foo
{
void Test(const int bar);
};
void Foo::Test(const int bar)
{
m_Bar = bar;
}
1. right-click on m_Bar
2. select Refactor -> Create From Usage
Actual result:
The suggestion defaults to:
"Member in Foo"
Declare member: "const int m_Bar"
Better result:
Declare member: "int m_Bar" <-- (i.e. no const)
i.e. do not use const for non-pointers/-references as the default suggested variable. |
|
|
feline
Whole Tomato Software
United Kingdom
18939 Posts |
Posted - Jan 18 2011 : 6:25:54 PM
|
Out of interest why do you feel VA should remove the "const" in this situation?
VA is doing its best to work out the correct type for the default suggestion, which can be edited before you accept it. For me the 4th radio button is "Parameter to Test", and personally I make parameters const when ever possible.
So I would think there are times when keeping the const is a good idea. |
zen is the art of being at one with the two'ness |
 |
|
|
Luke1410
Senior Member
32 Posts |
Posted - Jan 19 2011 : 04:05:10 AM
|
quote:
Originally posted by feline
Out of interest why do you feel VA should remove the "const" in this situation?
Sry for not being clear enough... I did not mean that VA should remove the const. What I mean is that atm when right-clicking on m_Bar and choosing to create "m_Bar" from usage as a member in Foo (not as a parameter to the method Test!), the resulting class looks like this:
class Foo
{
const int m_Bar;
void Test(const int bar);
};
What I like it to have been instead is:
class Foo
{
int m_Bar; <--- note, the member should be non-const
void Test(const int bar); <--- parameter is untouched --- i.e. still const
};
The reasoning is that a const member variable (that is, unless it's a const-pointer or const-reference) makes only sense in cases where the value is set upon construction and then no longer modified (i.e. a fixed member) which should only be used in very rare cases in the real world. Hence in the use-case described above I'd argue that VA can safely make the assumption that the generated member variable is meant to be non-const. |
Edited by - Luke1410 on Jan 19 2011 04:11:06 AM |
|
|
|
feline
Whole Tomato Software
United Kingdom
18939 Posts |
Posted - Jan 24 2011 : 12:01:42 PM
|
This makes sense, but I am wondering how this would work.
If VA leaves the "const" in the edit field in the Create From Usage dialog, then the code inserted is not the code shown in the dialog, which does not sound right.
VA could add or remove the const in the edit field as you press the radio buttons to change what you are creating, but then this would have to monitor if the user ever edited the text themselves. Since if the user ever edits the text, VA should not modify it any more, since that would risk undoing the users edit.
This might work smoothly, but I am a little concerned it will surprise people, and not be "obvious". Are you thinking of it working in a different manor to this? |
zen is the art of being at one with the two'ness |
|
|
|
Luke1410
Senior Member
32 Posts |
Posted - Jan 25 2011 : 02:15:44 AM
|
quote:
Originally posted by feline
VA could add or remove the const in the edit field as you press the radio buttons to change what you are creating, but then this would have to monitor if the user ever edited the text themselves. Since if the user ever edits the text, VA should not modify it any more, since that would risk undoing the users edit.
I was merely imagine it exactly like that.
A way to make it a bit more obvious might be to slightly redesign the create from usage dialog to something like this:
In principle VAX would only update the suggested default and keep the user-override in-sync with the updated default as long as the user hasn't manually edited the entry. |
|
|
|
feline
Whole Tomato Software
United Kingdom
18939 Posts |
Posted - Jan 25 2011 : 3:17:27 PM
|
This is an interesting idea, and certainly one solution. I have put in a feature request, to see what our developers make of this:
case=54480
I do like the idea, and I see the appeal here, but at the same time I have some concerns about how this would work in practice. |
zen is the art of being at one with the two'ness |
|
|
|
support
Whole Tomato Software
5566 Posts |
Posted - Mar 17 2011 : 11:47:56 PM
|
| case=54480 is fixed in build 1845 |
|
|
| |
Topic |
|
member-const handling for CreateFromUsage
