Sign in to follow this  

C++ private vs. public

This topic is 2546 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

My book says that one should try to keep all member variables in private in a class. And then you public functions to get them outside the class. I was just wondering what the argument for that is. I've read that it's suppose to be "safer", but I dont understand in what way it's safer and why.

After what I've learned a class should look like this:


class Object
{
public:
Object(int x, int y, int width, int height, ObjectType type, int ID, bool stati = true);
virtual ~Object();

virtual void draw(void);
virtual void update(float dt);

void updateRect(void);

int getX(void) {return mX;};
int getY(void) {return mY;};
int getHeight(void) {return mHeight;};
int getWidth(void) {return mWidth;};
int getID(void) {return mID;};
int getType(void) {return mType;};
int getStatic(void) {return mStatic;}
RECT getRect(void) {return mRect;};

private:
IDirect3DTexture9* mTexture;
RECT mRect;
int mX;
int mY;
int mWidth;
int mHeight;
int mID;
ObjectType mType;
bool mStatic;
};




I also will have to 8 more setXXX(...) functions in order to change the member variabels. If the member variables would have been public then I would have 16 less functions to worry about ... Please explain why I shouldn't do it that way.

Thanks,
simpler [smile]

Share this post


Link to post
Share on other sites
The thinking goes something like this:
Imagine your class has an "area" public member. Now you and your team use the class for a few weeks and in a bunch of places. Then you want to change it to calculate the area instead to save memory - minor nightmare to change all the dependent code. This is even worse if you created a library that people you've never met are using.

Another case is if you had a "height" member, and only weeks later do you realize that if someone puts in a negative value it will corrupt all savegames (or something equally horrible). Now you need to add a check for this case, which requires a method and you have a similar issue to above.

You also have the ability to create read-only members by only implementing a getter, or even a write-only.

Btw, creating getters and setters is so common that most IDEs have a shortcut for creating them. You might want to check if yours does.

EDIT: Also, many modern languages have solved this issue with properties, which have syntax like member variables but are actually methods - C++ doesn't have anything equivalent.

Share this post


Link to post
Share on other sites
You really have to question why you are using an object in the first place. If all you are doing is creating mindless setters and getters, it should be a struct, not a class. When building classes, you should be caring about the overall behavior of the object, not its internal components. The class uses its internal data to achieve a higher motive. Make sense?

Share this post


Link to post
Share on other sites
A class should never open its inner workings to outside code, regardless of how simple it might appear. Even basic a basic 'getter' and 'setter' allow you a stronger degree of control than just accessing a variable directly. For example, as soon as a variable is public, it can be both set and retrieved. Turning to methods allows you to decide on possibly including only a 'getter' and NOT a 'setter'.

It also allows for your class the flexibility to change. What if you realized that you need to track every time a variable is requested? If you have a 'getter', you can simply add a line of code in that method to do your tracking, with any other code none the wiser to your change. Doing this to a public variable would be much more difficult.

Imagine you have one variable, dollarsPerHour in your initial design. You then realize down the road that you want to instead have totalDollars and timeWorked.
If you originaly created a getter, getDollarsPerHour, you will simply need to change the method to use the two variables instead of just one. One line of code changed and any code that uses it is none the wiser to your change. If you had used a public variable, imagine trying to rework all of the code that uses your class, and imagine how messy it might get.

Those are just a couple reasons and examples I see.

Share this post


Link to post
Share on other sites
Objects have behaviour, not data. When you think about an object you shouldn't have to worry about it's internals. Ideally you write the interface that makes sense at the conceptual level, and shouldn't depend on the exact implementation.

Objects move(), they don't setX() or setY(). Objects generally don't change size (unless you are writing Mario or something). If they do then they will have grow() or shrink() member functions, not setWidth() and setHeight(). Objects don't change types, just create a new Object instead. Objects don't generally change from being static to non-static.

Also, don't store the object rectangle as a member variable. Instead of calling updateRect() and getRect(), simple make getRect() dynamically generate the rectangle. That, or only store the rectange and drop the x,y,width,height member variables.

Why is it safer? Well, Objects have these things called "invariants". An invariant is something that is a predicate that is always true. For example, you might say that an objects width and height are always greater than 0. The objects texture might never be NULL.

By making these variables private, you can vastly reduce the amount of code that accesses them, which makes enforcing this invariant easy. Other invariants are those I've mentioned about keeping certain state immutable. You could of course enforce this using the "const" keyword, but that can make it hard to write copyable objects.

A class without invariants isn't a class, it is just a convenient grouping of data.

Share this post


Link to post
Share on other sites
Quote:
Original post by rip-off
Objects move(), they don't setX() or setY(). Objects generally don't change size (unless you are writing Mario or something). If they do then they will have grow() or shrink() member functions, not setWidth() and setHeight(). Objects don't change types, just create a new Object instead. Objects don't generally change from being static to non-static.
QFE.

Most of the time, when you write a class that significantly features get/set methods, you are really writing a Facade.

Share this post


Link to post
Share on other sites
Thanks everyone for great answers!
Quote:
Original post by rip-off
Objects move(), they don't setX() or setY(). Objects generally don't change size (unless you are writing Mario or something).


You are right about, when I think about it I really don't need them.

Quote:
Original post by rip-off
Objects don't change types, just create a new Object instead. Objects don't generally change from being static to non-static.


Hmm I feel a bit confused. Lets say I inherit Enemy from object, shall I then just use


mStatic = false;
mType = NORMAL_ENEMY;




and if I inherit a Platform


mStatic = true;
mType = PLATFORM;




Okey, while I wrote that I realised how logical it was, I really dont need the arguments in the ctor [smile].
Because that is how you would do it, right?

Quote:
Original post by rip-off
Also, don't store the object rectangle as a member variable. Instead of calling updateRect() and getRect(), simple make getRect() dynamically generate the rectangle. That, or only store the rectange and drop the x,y,width,height member variables.


I allways felt that it was messy having both x,y,width,height and a rect but didn't know what to do about it. Thanks for telling me!

Share this post


Link to post
Share on other sites
Well, I would avoid storing the type as an enumeration, as the derived class itself can usually do the work if you design it properly.

For example, instead of:

Object &object = /* ... */
if(object.type == FOO)
{
fooStuff(object);
}
else if(object.type == BAR)
{
barStuff(object);
}
else if(/* ... */)
{
// ...
}



Could be:

class Object
{
virtual void foo() = 0;
};

// Derived classes implement foo() in specific ways.

object.foo();


You might find double dispatch a good solution for more complex situations, such as collision detection. It works well when the total number of classes isn't massive. But you generally can reduce the total number of classes with a data driven approach and by moving certain behaviour into auxiliary helper classes.

It depends on what you want to do with the "type" member, but building your own type system that isn't in sync with the regular type system can lead to bugs.

Share this post


Link to post
Share on other sites
The main reason why I have a type member is for collision detection. On a collision the player should check the mType value of the object and then act accordingly.

I don't understand why you don't think I should store a enum representing the type and setting it in the constructor.

But the mStatic you believe is fine to set in the constructor? I will use the mStatic to find out whether or not to update the object. If it is static I only need to draw it.

Share this post


Link to post
Share on other sites
then how about adding the object to an update list only if it needs updating? sort of the reverse approach: don't store it in the object, but have lists of objects "of a certain feature".

Share this post


Link to post
Share on other sites
Quote:
Original post by davepermen
then how about adding the object to an update list only if it needs updating? sort of the reverse approach: don't store it in the object, but have lists of objects "of a certain feature".
This!

Also, there's a case to be made along the following lines: If something doesn't update then maybe it shouldn't be derived from Object - since inheritance models an "is-a" relationship and because Object is something that updates (I know this because Object has, in its interface, an update() member) - It should follow that all derived classes are also things that update.

Share this post


Link to post
Share on other sites
Quote:
Original post by davepermen
then how about adding the object to an update list only if it needs updating? sort of the reverse approach: don't store it in the object, but have lists of objects "of a certain feature".


That sounds great!
I think my level class will look like this now:


class Level
{
private:
std::vector<Object*> mStaticObjectList; // objects that doesnt need to update
std::vector<Object*> mDynamicObjectList; // objects that need to update

...
};




Quote:
Original post by dmatter
This!

Also, there's a case to be made along the following lines: If something doesn't update then maybe it shouldn't be derived from Object - since inheritance models an "is-a" relationship and because Object is something that updates (I know this because Object has, in its interface, an update() member) - It should follow that all derived classes are also things that update.


You have a good point. But I don't know how strictly I have to follow the "is-a" rule.. Because something that doesn't move is also a Object. What I can think of doing is making a purely abstract class called Object and from that class I inherit StaticObject and DynamicObject. If you really are going to do it "the right way" that is how it should be done, right? But imo it feels fine to not call the update() function for any Objects in the mStaticObjectList. What are your thoughts?

EDIT: Eff, I'm changing in my saveLevel() function right now. I can't find out how to do it without knowing the type of the Object. There's something wrong with having a mType member?

[Edited by - simpler on December 24, 2010 4:35:38 PM]

Share this post


Link to post
Share on other sites
Quote:
You have a good point. But I don't know how strictly I have to follow the "is-a" rule.. Because something that doesn't move is also a Object. What I can think of doing is making a purely abstract class called Object and from that class I inherit StaticObject and DynamicObject. If you really are going to do it "the right way" that is how it should be done, right? But imo it feels fine to not call the update() function for any Objects in the mStaticObjectList. What are your thoughts?
I only skimmed the thread, but I'd recommend at least looking into component-based designs as an alternative to an inheritance-based hierarchy.

Component-based designs can be a little harder to get your head around at first, but IMO the benefits can be worth the extra effort. The questions you seem to be asking point directly to the problems inherent in a complex and/or deep inheritance hierarchy. The key issue, IMO, is orthogonality. In developing an inheritance-based hierarchy, sooner or later you're likely to encounter one or more 'diamond-like' relationships that are difficult to resolve in a sensible way. Component-based systems, on the other hand, are orthogonal by nature, meaning (among other things) that those sorts of problems rarely come up.

Anyway, that's just based on my own experience using component-based systems in my own projects and in third-party engines. (YMMV, of course.)

Share this post


Link to post
Share on other sites
Quote:
Original post by simpler
What I can think of doing is making a purely abstract class called Object and from that class I inherit StaticObject and DynamicObject. If you really are going to do it "the right way" that is how it should be done, right?
That's one way of doing it - the problem is if you go down that route, applying the principle to other things too, then you end up with a complex inheritance hierarchy that's rigid and horrible to extend.

Quote:
What are your thoughts?
I think that updating is too important a responsibility to leave up to the object being updated [wink]
I'd separate the tasks and finish up passing an updatee to an updater. Obviously the updaters will always be 'things that update', whilst entities are updated (or not) depending on whether they're passed to an updater. With this separation of responsibility it's very clear how objects are managed with respect to updates, it also permits the possibility of changing the type of updater for an updatee because of the separation.

Share this post


Link to post
Share on other sites
Quote:
Original post by jyk
Quote:
You have a good point. But I don't know how strictly I have to follow the "is-a" rule.. Because something that doesn't move is also a Object. What I can think of doing is making a purely abstract class called Object and from that class I inherit StaticObject and DynamicObject. If you really are going to do it "the right way" that is how it should be done, right? But imo it feels fine to not call the update() function for any Objects in the mStaticObjectList. What are your thoughts?
I only skimmed the thread, but I'd recommend at least looking into component-based designs as an alternative to an inheritance-based hierarchy.

Component-based designs can be a little harder to get your head around at first, but IMO the benefits can be worth the extra effort. The questions you seem to be asking point directly to the problems inherent in a complex and/or deep inheritance hierarchy. The key issue, IMO, is orthogonality. In developing an inheritance-based hierarchy, sooner or later you're likely to encounter one or more 'diamond-like' relationships that are difficult to resolve in a sensible way. Component-based systems, on the other hand, are orthogonal by nature, meaning (among other things) that those sorts of problems rarely come up.

Anyway, that's just based on my own experience using component-based systems in my own projects and in third-party engines. (YMMV, of course.)


Okey, great information there! It just feels a bit above my head right now. I'll stick to what I thought about first and maybe later on when i get the hang of inheritance hierarchy I'll try out the component-based way [smile]

Share this post


Link to post
Share on other sites
Quote:
Original post by dmatter
I think that updating is too important a responsibility to leave up to the object being updated [wink]
I'd separate the tasks and finish up passing an updatee to an updater. Obviously the updaters will always be 'things that update', whilst entities are updated (or not) depending on whether they're passed to an updater. With this separation of responsibility it's very clear how objects are managed with respect to updates, it also permits the possibility of changing the type of updater for an updatee because of the separation.


Err you make it sound so complex ^^ I'll try some stuff out and see if I get stuck on the road [smile]

Share this post


Link to post
Share on other sites

This topic is 2546 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this