Whole Tomato Software Forums
Whole Tomato Software Forums
Main Site | Profile | Register | Active Topics | Members | Search | FAQ
 All Forums
 Visual Assist
 Feature Requests
 'Encapsulate Field' issue&improvement

You must be registered to post a reply.
Click here to register.

Screensize:
UserName:
Password:
Format: BoldItalicizeUnderlineStrikethrough Align leftCenterAlign right Insert horizontal ruleUpload and insert imageInsert hyperlinkInsert email addressInsert codeInsert quoted textInsert listInsert Emoji
   
Message:

Forum code is on.
Html is off.

 
Check to subscribe to this topic.
   

T O P I C    R E V I E W
azdroppy Posted - Dec 18 2015 : 1:56:12 PM
Lets say we have:

class Foo
{
public:
Foo(std::string name)
: _name{ std::move(name) }
{
}
private:
std::string _name;
};

After applying 'Encapsulate field' to _name member resulting code looks the following way:

class Foo
{
public:
Foo(std::string name)
: Name(){std::move(name)}
{
}
std::string Name() const { return _name; }
void Name(std::string val) { _name = val; }
private:
std::string _name;
};

1. Name(){std::move(name)} - is incorrect
2. void Name(std::string val) { _name = val; } - is sub-optimal. It would be great to have a possibility to configure default behavior - either pass argument as const ref (if the object being encapsulated is a class) or use _name = std::move(val) if the object is movable.

Due to the issues mentioned 'Encapsulate Field' is not usable now as it requires too many manual code changes after apply
3   L A T E S T    R E P L I E S    (Newest First)
sean Posted - Mar 16 2016 : 12:14:15 AM
case=93992 is fixed in build 2093
feline Posted - Dec 19 2015 : 10:44:50 PM
Thank you for the clear example, I am seeing the problem with the constructor being updated incorrectly, and I have put in a bug report for this:

case=93992

When you trigger Encapsulate Field, the bottom portion of the Encapsulate Field dialog shows you a list of all of the references that are going to be updated. If you can easily spot the problem constructor initilizer list entry in the list, you can simply untick it in this list, and then this reference won't be updated, but the other changes will be made. Or you can simply untick the project or file nodes, to stop any code updates, just leaving you with VA generating the new functions, but not actually calling them.


For your second point, if you look in the VA Snippet Editor, set the type to Refactoring, and look at the Encapsulate Field snippet, you can make some edits to the code that VA will generate. You can certainly add const to the setter here. Adding "var" here might be unwise, since any changes you make here will always be applied. I cannot think of any reason not to always make the setter parameter const, so this change seems safe enough.

Accessing the snippet editor - http://docs.wholetomato.com/default.asp?W392
the new snippet editor - http://docs.wholetomato.com/default.asp?W476

We are considering passing items by reference for Encapsulate Field, but need to be careful, so that we are reliable in deciding when it is correct to do this:

case=7382

Working out if the object is movable, is there actually an easy way of working this out? I have done a bit of searching for this, since it's not something I know about, but am disappearing down a rabbit hole here.

The concept makes sense, but it does not seem that its safe to just blindly add std::move to everything, but I am not seeing anything so far to help guide me in working out when it is safe to use.


For your second post, you can add the extra const to the getter by simply editing the Encapsulate Field snippet, as above

I am not sure about the "constexpr" but though. As soon as you consider template types, you can have multiple constructors. If you start thinking about std::move, then it may not be just the main class constructor you need to consider either. I don't want VA adding something that it should not be adding, so we would need to be sure.
azdroppy Posted - Dec 18 2015 : 1:57:53 PM
Forgot to mention third issue:
std::string Name() const { return _name; }

it would be great to have an option to generate

const std::string& Name() const noexcept { return _name; }

instead or even

constexpr std::string Name() const noexcept { return _name; }

if the class ctor is 'constexpr' too

© 2023 Whole Tomato Software, LLC Go To Top Of Page
Snitz Forums 2000