Whole Tomato Software Forums
Whole Tomato Software Forums
Main Site | Profile | Register | Active Topics | Members | Search | FAQ
 All Forums
 Visual Assist
 Technical Support
 "Encapsulate field" refactoring breaks code

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
MichaelH Posted - Feb 26 2019 : 11:12:09 AM
The "Encapsulate field" refactoring creates (fortunately) non-compiling code in some cases, and (unfortunately) compiling code with different behaviour in others:

class Foo {
  public:
    int m_value;
};

class Bar {
  public:
    Foo m_foo;
};

void baz() {
    Bar bar;
    bar.m_foo.m_value = 13;
}

Trying to encapsulate m_value in Foo works fine.
However trying to encapsulate m_foo in Bar creates the following code:
class Bar {
  public:
    Foo getFoo() const { return m_foo; }
    void setFoo(Foo val) { m_foo = val; }
  private:
    Foo m_foo;
};

void baz() {
    Bar bar;
    bar.getFoo().m_value = 13;
}

Fortunately bar.getFoo() is not a lvalue, which prevents direct assignment, so compilation fails. However if we previously encapsulated m_value in Foo we end up with
void baz() {
    Bar bar;
    bar.getFoo().setValue(13);
}

Which does compile but has vastly different behaviour (m_value is set in the copy of Foo returned by getFoo(), leaving the value in the "real" Foo unchanged)

VA version 10.9.2318.0 built 2019.02.17
4   L A T E S T    R E P L I E S    (Newest First)
MichaelH Posted - Mar 01 2019 : 04:50:34 AM
@feline:
Happy to be of assistance.

@Dusan:
A good suggestion, but returning const references to class members is something I usually avoid (unless I explicitely want to return a const reference to that value), since we are rather used to functions returning results by value (i.e. getFoo() returning Foo and not const Foo&)

That said, this still can be a very useful way to detect (most of) the problematic spots for the time being (as calling non-const functions on the const reference causes compiler errors).
Dusan Posted - Feb 28 2019 : 6:06:06 PM
You can modify the snippet with name Encapsulate Field to return a const reference if you like...
feline Posted - Feb 26 2019 : 3:08:39 PM
And it can also go wrong in a very similar way when the member variable is being passed as a parameter that is actually a reference. I just wonder how come this never came up before. At least we are now aware of this problem.
feline Posted - Feb 26 2019 : 3:02:07 PM
Ouch, that is rather unpleasant. Thank you for pointing this out, I have put in a bug report for this:

case=137236

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