Getter vs public member and the possibility to replace the returned object [closed]

Question

Closed. This question is opinion-based. It is not currently accepting answers.

---

Want to improve this question? Update the question so it can be answered with facts and citations by editing this post.

Closed 8 years ago.

Improve this question

I recently had a debate with a co-worker about getter & setter which use pointers against a public member pointer (the topic results in the same debate without pointers and instead having a getter returning a reference). I'm not sure if this topic will be a duplicate, but while searching for an answer to the topic I found just three topics ( here, here and here) somehow related to the topic "getters/setters vs public member" without pointing into the direction as our debate went.

So here is the situation:

There is a class Text. Objects of type Text always contain another object of type TextProperties (e.g. size, color, bold, italic, underline ect). To modify a property I wanted to do it directly using the methods of the TextProperties-object without having the need to create a new object. This finaly resulted in a getter returning a pointer of the TextProperties-object.

Here a very simple example of the situation:

//example for TextProperties
class TextProperties
{
protected:
  int m_size;
  bool m_bold;

public:
  bool Bold() { return this->m_bold; }
  void Bold(bool bold) { this->m_bold = bold; }
  int  Size() { return this->m_size; }
  void Size(int size) { this->m_size = size; }

  TextProperties()
  {
    this->m_bold = false;
    this->m_size = 5;
  }

  ~TextProperties() {}
}

//example for Text
class Text
{
protected:
  TextProperties* m_properties;

public:
  TextProperties* Properties() { return this->m_properties; }
  void            Properties(TextProperties* properties) { this->m_properties = properties; }

  Text()
  {
    this->m_properties = new TextProperties();
  }

  ~Text()
  {
    delete this->m_properties;
  }
}

//the usage as I want it:
int main()
{
  Text* exampleText = new Text();

  //easily and short a property gets changed
  exampleText->Properties()->Bold(true);

  delete exampleText;
}

My co-worker argued against this solution because with that solution the door is opened to replace the object (also when using a reference-returning getter) while bypassing any setter.

//An example for the replacing bypassing the setter:
int main()
{
  Text* exampleText = new Text();

  //easily and short a property gets changed
  exampleText->Properties()->Bold(true);

  TextProperties* test = exampleText->Properties();
  *test = *(new TextProperties());
  //at this point the text wouldn't be bold

  delete exampleText;
}

The only possibly safe solution (where the object can't be replaced with another) is as far as I know making the return const and forcing anyone who uses this class to create a new TextProperties-object for changing just one property. As mentioned this kind of solution is not wished.

//just as intimation how the other, replacing safe 
//way of usage I want to avoid could look:
int main()
{
  Text* exampleText = new Text();

  //if needed copy-construct the object otherwise just use (default) constructor
  TextProperties* newProperties = new TextProperties(exampleText->Properties()); 
  newProperties->Bold(true);
  exampeText->Properties(newProperties);

  delete exampleText;
}

So far the situation - now the debate:

Accepting the goal to be able to modify the properties directly my co-worker than mentioned that the solution in his opinion has no difference to a public member.
My point of view is as follows:

• with the getter/setter the internals are hidden (=encapsulation)
  • internal changes could be performed easier/faster as with public-member accesses scattered over all the source code
  • also possibly ugly internal names with prefixes and/or suffixes are covered with a nice and easily describing name (e.g. internal: int m_x, m_y <-> external: Width(), Height() (yep people do things like that)) making code using the class more clear and easier to understand
• providing getter/setter the class-consumer can be sure that things are done in the way they should be done (public member for me only would make sense for simple types as int ect. since more complex objects maybe could need some specific actions e.g. for ensuring the correctness of the contained information) also the member is protected against modifications by mistake.
• if someone whants to replace the object while bypassing a getter he has to do it deliberate and thus it is his fault when the code does not work as expected, my task at this point only consists of ensuring that no corruption can happen (but that is anyway all the time my task) -> if you want force some things you can do... good or bad that's another question with that c++ does not deal, it just gives the possibilities ;)
• when avoiding by value objects don't need to be copied which can be but has not to be a performance gain (point added by edit)
• when accessing the object in editable way und change the members of the accessed object directly it is (for me) shorter und comprehensible source code (point added by edit)

I think the first two points are mainly arguments from the classic getter/setter vs public member discussions. Anyhow this arguments didn't change the opinion of my co-worker and I'm now brooding for days on this topic. So I'd like to know is there really no difference (I personally would definitely say "there is a difference") and the arguments are just my subjective opinion, is it a question of personal taste, do exist more convincing arguments (for both opinions) or exist completely different solutions which wouldn't cause this discussion?

Edit: additional to the questions I want to point more on the focus of my question (according to the first comment on the answer) and specify the question itself:
If the decision hast been made to have getters & setters and also breach the encapsulation with getters using reference or pointers: Why could/should this still be better than making the member a public member? (I tried to provide with the list arguments for having these getters and I'd like to know which of these arguments is in/valid or which other arguments could be pro/contra these getters or public member)

In hope that this is not a duplicate I expect some interesting answers. Thanks! :)

Answer 1

If you want to achieve the following two goals:
1) Good object-oriented design.
2) Complete avoidance of unauthorized changes (bypassing incapsulation) of the internal TextProperties object.

I would suggest the following solution:
1) Define an interface TextPropertiesInterface for getting/setting individual text properties. It will look similar to TextProperties class.
2) Implement TextPropertiesInterface e.g. by a nested private class within Text class (other possible solutions are at the end of the post). That implementation will actually read/change the internal TextProperties object.
3) Expose the internal implementation of TextPropertiesInterface from Text class as a reference/pointer to the interface.

This is how I see it in the code:

class TextPropertiesInterface{
public:
    virtual int GetSize() = 0;
    virtual void SetSize(int iSize) = 0;
    virtual Color GetColor() = 0;
    virtual void SetColor(Color oColor) = 0;
    etc ...
}

class Text{
private:
    // internal object completely protected from bypassing incapsulation
    TextProperties m_Properties;

    // Nested private implementation of the interface
    class TextPropertiesInterfaceImpl: public TextPropertiesInterface{
        TextProperties& m_Properties; // init it with Text::m_Properties
        ...
    }

    TextPropertiesInterfaceImpl m_TextPropertiesInterfaceImpl;

public:
    // Exposing the interface outside
    TextPropertiesInterface& Properties(){
        return m_TextPropertiesInterfaceImpl;
    }
};

Now the clients/users of Text class can get/set text properties in such a way TextObj.Properties().SetWidth(10)
And there is no way for the external clients/users to modify the internal TextProperties object in unauthorized manner (e.g. by replacing the object etc).

Additional note: You can implement TextPropertiesInterface in some other ways that may require less lines of code.
Example 1: Class TextProperties can implement TextPropertiesInterface. That will eliminate class TextPropertiesInterfaceImpl but will reveal some implementation details to external clients/users of Text class. Also it will allow such a dirty trick:
static_cast<TextProperties&>(TextObj.Properties()) to bypass incapsulation.

Example 2: Class Text can implement TextPropertiesInterface using private inheritance in such a way class Text: private TextPropertiesInterface { ... } but it has similar drawbacks.

Answer 2

Some of the potential problems with returning a pointer or reference to a private member:

• As you mention, encapsulation is breached. You have given access directly to the member, despite the fact it is private.
• If the Text class keeps some invariants over its TextProperties member, those may be invalidated by modifying the object from outside the class.
• The client code must be aware that the reference is only valid while the Text object that returned it is still alive.

As your friend says, these would all be true if you just make the member public. However, maybe you want to do some extra work around simply returning the member (such as maintaining those invariants).

Sometimes these are necessary issues to deal with if you really want the interface as you described. For an example of this in practice, take a look at std::vector::operator[] - it returns a reference to the element. Note that the semantics of this function and yours are different, however - std::vector is a container, so operator[] is exposing the elements that are contained within (just like using [] on an array would). Instead, you are exposing some state of your Text object.

If I had to pick between them, I would always recommend using the reference returning getter over returning a raw pointer (unless its possible for there to be no TextProperties, in which case you'd return nullptr (or use a boost::optional)).

If acceptable, however, the safest and cleanest interface is to simply return the member by value and also have a setter that takes it by value. This should be your default approach, but you may not want to do this if copying the object around may be unnecessarily expensive.

Just think about the tradeoffs and pick a design that best suits your class. Honestly, I can't see a reason why you wouldn't just get and set by value.