Public Data Members VS Getters, Setters

Public Data members vs Getters, Setters

Neither. You should have methods that do things. If one of those things happens to correspond with a specific internal variable that's great but there should be nothing that telegraphs this to the users of your class.

Private data is private so you can replace the implementation whenever you wish (and can do full rebuilds but that's a different issue). Once you let the Genie out of the bottle you will find it impossible to push it back in.

EDIT: Following a comment I made to another answer.

My point here is that you are asking the wrong question. There is no best practice with regard to using getters/setters or having public members. There is only what is best for your specific object and how it models some specific real world thing (or imaginary thing perhaps in the case of game).

Personally getters/setters are the lesser of two evils. Because once you start making getters/setters, people stop designing objects with a critical eye toward what data should be visible and what data should not. With public members it is even worse because the tendency becomes to make everything public.

Instead, examine what the object does and what it means for something to be that object. Then create methods that provide a natural interface into that object. It that natural interface involves exposing some internal properties using getters and setters so be it. But the important part is that you thought about it ahead of time and created the getters/setters for a design justified reason.

Public variables bad practice vs Getters and Setters functions?

First of all, a struct is completely equivalent to a class, but with the default member access being public rather than private.

Now, in Object Oriented Programming (OOP), it's not considered good practice to have public data members (variables), because that makes all your code dependent on the internals of the class, and thus breaking a primordial principle of OOP, and that is...

Holy and Sacred Encapsulation

Encapsulation is the coding philosophy that states that a class should englobe both data and the code that manages it in a single tight entity. That is, you don't access data directy, but rather you use methods from the class to manipulate such data. This has several design advantages, such as that you'll know that no code except the one inside the class may incorporate bugs with respect to the manipulation of such information.

Now, get()ers and set()ers, otherwise known as accessors, are a complete lie! With accessors, you're tricking yourself into thinking that you're respecting encapsulation, when you're rather breaking it! It adds bloat, unnecessary verbosity, bugs, and everything but encapsulation. Instead of having a class Person with unsigned getAge() and void setAge(unsigned), have it with a unsigned getAge() and a void incrementAge() or however you want to call it.

Now, to your question's core...

"Plain old" structs

Encapsulation is not always desired. Although you should (usually) not do this on header files (again, for at least some bit of encapsulation), you may create static plain old structs that are private to a single translation unit. My recommendation is to make them even "older" than they already are, i.e...

  • All data members are public.
  • No methods.
  • No constructors (except implicit ones).
  • Inheritance is always public, and only allowed from other plain old structs.
  • I repeat, don't put them on header files!

Now, another use for plain old structs is (ironically) metaprogrammatic exporting of constexpr data and types, otherwise known as modern-hardcore-template-metaprogramming-without-having-to-type-public-everywhere, for example...

template<bool B, typename T>
struct EnableIf {};

template<typename T>
struct EnableIf<true, T> {
typedef T type;
};

template<bool B, typename T>
using SFINAE = typename EnableIf<B, T>::Type;

Inline getter and setter vs public variables

The compiler will most probably inline these functions, and there will be no function call overhead. I would avoid both getter/setter and public member variabless and think why these are used and provide a function to do that in that class. Most of the getter/setter/public member variabless can be removed this way.

Are public variables faster than using getters and setters?

Will creating a setter and getter function for each of these variables impact performance? (There will be at least 100 instances of this class at a given time)

Probably no. Theoretically yes, but in practice, the cost of calling an extra function to get a particular value is negligible. The only way it will impact performance is if you end up calling these methods all the time (say ... 50000 times per second).

Also should I create setters and getters?

Probably no. Good OO design follows a guideline called "tell, don't ask". Usually you should see what operations you need these variables for, then either implement those operations in the class, or implement them in a class that has access, or use a different model (visitor pattern comes to mind).

I heard public variables are really bad practice but there are a lot of variables, can this be an exception?

Having public variables is not bad practice. Having public variables when you have invariants on them, is bad practice.

For example, if you have a variable measuring the weight of an object, you will want to make sure this cannot be set to an invalid value (such as a negative amount, or a ridiculously large amount). If you make the variable public, you will either have to check the value you set everywhere in client code where you modify it, or give up on validating that value.

Both are bad, as they are errors that couldn't exist if you had a propper setter, with validation.

In short, having public variables is only acceptable if you have no invariants on them.

Considerations:

  • use public variables, if setting any value permitted by the variable type is OK, at any point (you have no invariants)

  • use private variables if you have invariants.

  • use public methods to define operations, not "access to internal variables".

  • design your public API in terms of operations you want to perform when you look at it from the client code, not in terms of variables present in the internal implementation.

In this context, getters and setters make sense some times (rarely), but not because you have the variables in the class.

  • getters and setters are a symptom of trying to solve a problem in the wrong place: if you have an operation X in class A that uses variables from class B, then you should probably define operation X in class B, then call it from class A.

Is there any real argument for getters/setters instead of public member variables in a simple Point class?

In C++, I will add setters and getters if only to conform to the Uniform Access Principle; why should the caller have to keep track of what is stored and what is computed?

When it comes to basic data types, where the only invariant is scope, I will happily admit that the extra effort might not be worth it. But something like a Range type (with .high, .low, .distance) where there is an invariant to be maintained (hight >= low, distance = high - low) then it's a necessity. Otherwise, all the clients of the type end up having to maintain the invariant when that should be the job of the type itself.

Encapsulation - Why I am using getter setter to make my data members public if I already declare them private in class

This is an excellent question. Often you see code examples that make members private but exposing them via a getter/setter pair, without the getter/setter doing anything else than setting the corresponding member.

In my book this is not encapsulation at all. You are no better of than just making the members public. Although a lot of people are uneasy to do that, they would happily provide accessors automatically for all their members.

One reason to do provide accessors is the ability to do input validation. E.g. if you empIds have a checksum, you could enforce it in the setter. Something that is not possible with direct access to the member.

In my opinion it would be better to think about the role this object will play and see how it can achieve that role with a minimum of accessors. Otherwise your code will probably violate the Law of Demeter.

Advantage of set and get methods vs public variable

What I have seen someday on SO, as answer (written by @ChssPly76) why to use getters and setters

Because 2 weeks (months, years) from now when you realize that your
setter needs to do more than just set the value, you'll also realize
that the property has been used directly in 238 other classes :-)

there are much more advantages:

  1. getters and setter can have validation in them, fields can't
  2. using getter you can get subclass of wanted class.
  3. getters and setters are polymorphic, fields aren't
  4. debugging can be much simpler, because breakpoint can be placed inside one method not near many references of that given field.
  5. they can hide implementation changes:

before:

private boolean alive = true;

public boolean isAlive() { return alive; }
public void setAlive(boolean alive) { this.alive = alive; }

after:

private int hp; // change!

public boolean isAlive() { return hp > 0; } // old signature
//method looks the same, no change in client code
public void setAlive(boolean alive) { this.hp = alive ? 100 : 0; }

EDIT: one additional new advange when you are using Eclipse - you can create watchpoint on field, but if you have setter you need just a breakpoint, and... breakpoints (e.g. in setter method) can be conditional, watchpoints (on field) cannot. So if you want to stop your debugger only if x=10 you can do it only with breakpoint inside setter.

Calling getters inside a setter function vs direct access to data members

Code in the class always has full acess to the internal data members (and member functions too). So it is not necessary. My thoughts on if you should do it

  • if the getters and particularly setters have side effects (imagine you keep a count of how many times a particular value is changed, or validate a value) then you should call them
  • the overhead when compiled for release will disappear since the compiler can see that you are just reading or writing the value (if they are simple read and write get/set)
  • you might get into the habit of always calling them just in case you later want them to have side effects

note I dont say whats 'best', just things to take into account

Getter vs public member and the possibility to replace the returned object

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.

Encapsulating data so the setter is private and getter is public

What about setting your Creator as friend to class EntityX:

   class EntityX
{
friend class Creator;
public:
EntityX(int x, int y) : x(x), y(y)
{}
int GetX() const {return x;}
int GetY() const {return y;}
private:
int x,y; // Effective C++ third edition, Item 22: Declare data members private
};

Update:

Or you could use templatized friend-ship, see code below:

#include <iostream>
#include <memory>

template<class T>
class EntityX
{
friend T;
public:
EntityX(int x, int y) : x(x), y(y) {}
int GetX() const {return x;}
int GetY() const {return y;}
private:
int x,y; // Effective C++ third edition, Item 22: Declare data members private
};

struct Creator
{
static const std::shared_ptr<EntityX<Creator>> create()
{
std::shared_ptr<EntityX<Creator>> entity = std::make_shared<EntityX<Creator>>(1,2);
entity->x = 1;
entity->y = 2;
return entity;
}
};

int main()
{
std::shared_ptr<EntityX<Creator>> const E = Creator::create();
std::cout << E->GetX() << ", " << E->GetY() << std::endl;

return 0 ;
}


Related Topics



Leave a reply



Submit