Whole Tomato Software Forums
Whole Tomato Software Forums
Main Site | Profile | Register | Active Topics | Members | Search | FAQ
User name:
Password:
Save Password
Forgot your password?

 All Forums
 Visual Assist
 Technical Support
 "Encapsulate field" refactoring breaks code
 New Topic  Reply to Topic
 Printer Friendly
Author Previous Topic Topic Next Topic  

MichaelH
Junior Member

10 Posts

Posted - Feb 26 2019 :  11:12:09 AM  Show Profile  Reply with Quote
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

Edited by - MichaelH on Mar 18 2019 11:15:15 AM

feline
Whole Tomato Software

United Kingdom
18724 Posts

Posted - Feb 26 2019 :  3:02:07 PM  Show Profile  Reply with Quote
Ouch, that is rather unpleasant. Thank you for pointing this out, I have put in a bug report for this:

case=137236

zen is the art of being at one with the two'ness
Go to Top of Page

feline
Whole Tomato Software

United Kingdom
18724 Posts

Posted - Feb 26 2019 :  3:08:39 PM  Show Profile  Reply with Quote
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.

zen is the art of being at one with the two'ness
Go to Top of Page

Dusan
Whole Tomato Software

Slovakia
177 Posts

Posted - Feb 28 2019 :  6:06:06 PM  Show Profile  Reply with Quote
You can modify the snippet with name Encapsulate Field to return a const reference if you like...
Go to Top of Page

MichaelH
Junior Member

10 Posts

Posted - Mar 01 2019 :  04:50:34 AM  Show Profile  Reply with Quote
@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).

Edited by - MichaelH on Mar 01 2019 06:03:36 AM
Go to Top of Page
  Previous Topic Topic Next Topic  
 New Topic  Reply to Topic
 Printer Friendly
Jump To:
© 2023 Whole Tomato Software, LLC Go To Top Of Page
Snitz Forums 2000