getter/setter vs public for multiple subclasses?

[homing]

Hey,

I'm not sure if I should go with public variables for mutliple subclasses in a class or with getter/setter

Right know I'm using getter/setter and avoid public at all, but i'm not sure if its the right approach and especiall in c++.

e.g i must be able to set and get the value of all my subclass variables:

getter/setter:

class MyClass
{
.
.
.
.
private:
 class Diamond
 {
public:
  Sprite* GetDiamondSprite(); //get/set through getter
  Animation* GetAnimation();
  Int GetValue();

  void SetValue(int value);

private:
   Sprite* diamond_sprite;
   Animation* animation;
   int value;
 };
 
 class WinText
 {
public:
   const String& GetText();
   const Font& GetFont();
   Animation* GetTextAnimation();
  
   void SetText(const String& text);
   void SetFont(const Font& font);

private:
   String text;
   Font font;
   Animation* textAnimation;
 };

 class WinBar
 {
public:
   Array<Diamond>& GetDiamonds();
   Animation* Getanimation();
   Sprite GetSprite();

private:
   vector<Diamond> diamonds;
   Animation* animation;
   Sprite* background;
 };

};

publics:

class MyClass
{
.
.
.
.
private:
 class Diamond
 {
   Sprite* diamond_sprite;
   Animation* animation;
   int value;
 };
 
 class WinText
 {
   String text;
   Font font;
   Animation* textAnimation;
 };

 class WinBar
 {
   vector<Diamond> diamonds;
   Animation* animation;
   Sprite* background;
 };

};

.....
Use subclasses in MyClass
.....

which one would be better?

[helios]

Modifying a class that's had its guts spilled all over the place is a nightmare in C++. This is the case with both dumb getters/setters and public variables. For certain classes this is acceptable (e.g. a Point2D), but very often it's not. If possible, try to redesign your code to avoid getters and setters altogether. E.g. instead of having sprite.set_pos(x, y), prefer sprite.move(x, y), or even sprite.move(Position(x, y)). You want to expose an interface that treats the type as a black box, not as a bag of variables.
If the above isn't workable for some reason, use getters and setters.
For certain types that will have very limited use, having everything public is sometimes acceptable.

[homing]

in this case its not possible to redesign it, because i'm using a framework where objects like Sprite, Animations can only be initialized in (for example) class MyClass. The main question is, if its ok to make such variables public, instead of using getter&setter.

Normally I would never make variables public, I would always prefer a Getter&Setter, but previously I've seen a lot of C++ code especially for such small subclasses where things are just public.

[coder777]

if its ok to make such variables public, instead of using getter&setter.

Yes. I that case it is unnecessary overhead. You gain nothing.

If you want the variables to be modified outside the class make them public. If you want to use them [mainly] internally make them private. And protected if you want to allow inherited classes to use them.

That's basically how this access specifier work.

[homing]

hmm, but somehwer i found this sentence:

Providing getters and setters for them breaks that encapsulation, but it's still better than public data members because there's only once access point to that data.

You'll notice this during debugging. If it's private, you know you can only modify the variable inside the class. If it's public, you'll have to search the whole code-base for where it might be modified.

[helios]

objects like Sprite, Animations can only be initialized in (for example) class MyClass.

What do you mean?

Yes. I that case it is unnecessary overhead. You gain nothing.

Dumb getters and setters have zero overhead due to function inlining. Don't make things public for performance reasons.

[homing]

What do you mean?

It actually doesn't matter, I just want to know, if it might be better to use publics in that case?, because I've seen a lot of code where other c++ programmers just make them public for such short private subclasses.

Dumb getters and setters have zero overhead due to function inlining. Don't make things public for performance reasons.

Yep that's true.

Somehow its more convenient to work with publics (in this case, where i have mutliple subclasses where variables need set&get access) and it also looks more readable, but I'm still not quite sure, about publics...
(code from my initial post above)

[helios]

I just want to know, if it might be better to use publics in that case?

Using public data members is never better, since there's nothing you can do with public members that you can't do with getters/setters. What you really want to know is whether you can get away with being a bit lazy right now without dramatically increasing the cost later on.

If the class is only going to be accessible from a handful of functions, I'd say that's acceptable. Keep in mind, though, that every usage of a class member potentially increases the cost of maintenance every time the class needs to change.

[homing]

Well the class is about 1.000 lines of code, won't be modified anymore.
No other class will use MyClass, so actually these publics inside the private subclasses are somehow private to the MyClass.

[MiiNiPaa]

If class is just behaviorless aggregate of data, it is fine to make it a class with all members public (often keyword struct is used to denote that). Otherwise you want private data: even if there is nothing which seems to suggest using accessors, it might change. For example:

There was a class called Bad representing entity which has a property foo and property bar, which can be calculated from foo using complex formula. It looked like this:

class Bad
{
  public:
    int foo;
    int bar() const { return do_complex_calculations(foo); }
}

Later it was found that users are requesting bar way more than accessing foo. More to that, users tends to have accesses to foo and bar to be separate from each other with calls to same property clumped together. Solution is simple: cache result of calling bar and recalculate it if it is dirty: wasn't calculated before or depended upon value changed. It would be simple if we could do additional actions when foo is accessed. Which we cannot, as it is public and modifiable directly by anyone. It was solved, but at what cost:

class Bad
{
  public:
    int foo;
    int bar() const
    { 
        if(foo == foo_prev) {
            return bar_cache; 
        } else {
            foo_prev = foo;
            return bar_cache = do_complex_calculations(foo);
        }
    }
  private:
    int foo_prev;
    int bar_cache;
}

Additional member (increasing size) and complex logic (reducing perfomance gain).
Now imagine if there several such members of complex types, increasing size and reducing perfomance drastically? It wouldn't be such problem if we used accessors from the beginning (even worst type which gives reference to innards):

class Bar
{
  public:
    const int& foo() const { return foo; }
    int& foo()  
    { 
        dirty = true;
        return foo; 
    }
    int bar() 
    { 
        if(dirty) {
            dirty = false;
            return bar_cache = do_complex_calculations(foo); 
        } else {
            return bar_cache;
        }
    }
  private:
    int bar_cache;
    bool dirty;
}

Simplier logic and less overhead (in actual use case dirty bytemask incurred no overhead as it was placed in padding bytes)

[helios]

won't be modified anymore

Sure, that's what we all say.

[homing]

@MiiNiPaa your are right, but in this case, these variables, always must provide Getter/Setter accessors, this will never change. The getter and setter function would always be one line functions there will never be anything else in it.

The Setter function is actually just used once inside at the initialization of the class, where the variable gets initialized, then I have to work through the getter directly with the variable.

You are still completely right, but it this one special case, where im just not sure, about.

[helios]

If you're not sure then just use getters and setters. Consider the cost of being wrong in either scenario.

[dhayden]

Helios writes:

Using public data members is never better, since there's nothing you can do with public members that you can't do with getters/setters.

Except taking the member's address, or creating a reference, or ++, --, +=, -=. /=, *=, or a=b=c type expressions or ....

While getters and setters have their place, I think they are overrated. In your case, they would trade certain complexity and loss of flexibility for uncertain future gain. To me, you should use getters/setters if there is a clear, present need for them, or if the cost of changing the data later is huge. Otherwise it's just decorating.

[helios]

Except taking the member's address, or creating a reference, or ++, --, +=, -=. /=, *=, or a=b=c type expressions or ....

Foo &get_foo();
const Foo &get_foo() const;

But IMO returning a non-const reference from a getter should be avoided whenever possible.

certain complexity and loss of flexibility

Any interface that exclusively exposes functions is less complex than any interface that exposes a single variable, precisely because of the removed flexibility.

loss of flexibility for uncertain future gain. To me, you should use getters/setters if there is a clear, present need for them, or if the cost of changing the data later is huge.

Code will need to be modified, lest it rots. This just a fact of life.
There is never a need right now to limit the power of an interface, just like there is never a need right now to keep the scope of a variable as small as possible. Yet I don't see you arguing in favor of making all data global, even though disallowing callees from modifying the data of callers at will limits the expressive power of the program.
We do these things because it makes managing the eventual complexity of the program easier, not because they're currently needed.

[dhayden]

Any interface that exclusively exposes functions is less complex than any interface that exposes a single variable, precisely because of the removed flexibility.

I find that it's good to make interfaces more flexible, not less. Otherwise there is inevitably some reasonable need for the class later on that I didn't think of and that the class doesn't provide.

returning a non-const reference from a getter should be avoided whenever possible

Amen to that!

Code will need to be modified, lest it rots. This just a fact of life.

I agree completely.

There is never a need right now to limit the power of an interface

I don't agree with this. My point is that you can usually tell "right now" if you should be using getters and setters. I work with code that I wrote almost 20 years ago. It's been modified constantly during that time and with *very* few exceptions, there's been no need to change the interfaces. Some classes use getters/setters, some don't. So my point is that you an and should use them when they are warranted, but blindly putting getters and setters in all your code just obfuscates it.

[helios]

Otherwise there is inevitably some reasonable need for the class later on that I didn't think of and that the class doesn't provide.

Expanding an interface is a non-breaking change. Modifying or restricting it isn't.

I don't agree with this. My point is that you can usually tell "right now" if you should be using getters and setters. I work with code that I wrote almost 20 years ago. It's been modified constantly during that time and with *very* few exceptions, there's been no need to change the interfaces. Some classes use getters/setters, some don't.

I disagree. I think you're trying to generalize from specific cases. As a counterexample, I have some code I wrote just six years ago that I wanted to modify and found that it was a herculean task because in a few key classes that are used everywhere in the codebase I had made everything public, so changing anything broke everything.

blindly putting getters and setters in all your code just obfuscates it.

I do agree with this. I argued that getters and setters should be avoided in lieu of a properly (or, at the very least, narrowly) designed interface that encapsulates the type.

[homing]

returning a non-const reference from a getter should be avoided whenever possible

true, but I forgot to tell you that it must be non-const, in my case.
the initialization somehow requires direct access.

[helios]

Then the initialization should be a responsibility of the inner class, not of the caller.