Sign in to follow this  
kzar

When are public variables in a class OK?

Recommended Posts

Hi, I am pretty new to c++ but I am learning it because I have to for uni. Anyway I am starting to get the hang of it but I have got a bit confused making my animation class's. Then I looked at the cone3D tutorial to see how he did it and I noticed to get variables between some of the class's he just made them public. Now until now I have be told by everyone to make all variables in a class private except very occasionaly, so I was wondering when you would use public variables? Normaly I could just make a get_bob() function to get the value of bob instead of making it public but when I have to get the value of a variable through two class's it gets a bit harder! Anyway thanks, David

Share this post


Link to post
Share on other sites
Make a variable public when the point of the variable in the class is just to have somewhere to store the data and make it accessible to anyone and everyone. However, in most cases that's not the purpose of a variable in a class.

Share this post


Link to post
Share on other sites
If the variable can be changed to any value at any time without affecting anything else, and it is information that the world at large needs access to, then making it a public variable is fine.

If you have a simple 3d vector, for instance, making the x, y, and z variables public is probably OK.

If you have a normalized 3d vector, though, they shouldn't be public. x, y, and z can not take on any random values, they can only take on points on the unit sphere.

CM

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
Quote:

Normaly I could just make a get_bob() function to get the value of bob instead of making it public but when I have to get the value of a variable through two class's it gets a bit harder!

How do you mean?

If ClassA contains an object ClassB, and ClassB contains an integer SomeValue, you could do this (providng you had the relevant get / set routines):

instanceOfClassA.getInstanceOfClassB().getSomeValue();

Although, IMO, get/set routines should only be used when they perform some validation or other operation on the data member being returned. If they contain nothing more than a return statement, it usually isn't worth it.

Share this post


Link to post
Share on other sites
C++ advocates will tell you to always make all variables private. But essentially it's only a protection to prevent the programmer from writing 'spaghetti code'. It's perfectly possible to have good code without any private variables, it's just harder (pure C code).

The most important thing is to encapsulate your implementations and only allow access through interfaces. If the code behind that interface uses public variables, but works fine, then that's not too bad. So it's important to see the distinction between implementation and interface. Many classes have both an interface part (public methods), and implementation part (functions and data, highly recommended to keep private). Other classes are purely interfaces (all virtual methods, no variables). And finally there are classes which are purely implementations (representing an algorithm).

So when exactly is it ok to have a public variable? Well, the rule of thumb is to always make every variable private, unless it's very cumbersome to use access methods and you're absolutely sure no encapsulation is needed now or in the future. One perfect example is a vector class with x, y, z variables. It's pretty useless to make these private (exceptions exist but are extremely rare).

Share this post


Link to post
Share on other sites
If the members being physically stored are essential to the concept, such as with ::std::pair instantiations, then it's usually okay to have the members be public. Such as with pairs, the whole point is just grouping together values of any type, providing full access to both, so making them public is generally fine. Even still, there are some times where it's beneficial to return a non-const reference to a private datamember from a member function, even if the member in question should be able to be accessed completely, such as when you need the same pure abstraction for working with both values and proxies.

A good example of this which I'm dealing with now is a library I'm developing for reprenting quantities with attached units. Often times, other libraries, such as graphics libraries, use raw values/arrays/other types to represent points and vectors in space, without explicitly attaching length units. Because of this, such data has to be stored in a raw form, however, it's nice to adapt iterators to such types or directly create proxies to individual elements which provide the added level of abstraction of attaching units and providing appropriate operator overloads, member functions, type/template members, etc.

Such proxies should have the same interface as quantities which directly contain their values so that they can all be used in expressions while using the same syntax. The fact that some are proxies and some are the actual types should be an implementation detail. Since the proxies can't contain the datamembers directly, a function returning a reference is the only option. Rather than having two different interfaces for the same concept, which would cause the problems above, the pure abstraction just involves a function returning a reference rather than having a public datamember. For the non-proxy, the datamember could still be public as well, but that would just be creating a redundant way to access the same data, so it's better not to.

So I guess the rule I'd say is, always prefer private members where you can, use public members if the concept of the encapsulating type requires that the type contains the data directly (which in-and-of-itself is an extremely rare occurrence), unless you need a more general abstraction with proxies or other similar high-level concepts, in which case a function returning a reference may be the better approach.

Share this post


Link to post
Share on other sites
Variables should be public when they are part of the interface and changing their value can't affect the validity of the object. In this case, accessors would only add unnecessary complexity to the interface.

For example, the x, y, and z components of a generic 3D vector should be public, while the x, y, and z components of a normal should not (because it must always have a length of 1).

Share this post


Link to post
Share on other sites
Quote:
Original post by dxFoo
I use public variables if retrieving them is a speed concern.

You shouldn't.

*edit: I'll be a little less terse. If a variable should be private, and you are making it public just for speed reasons, you're microoptimizing at a dangerous level. Your compiler is extremely smart, and is more than capable of inlining your accessors/modifiers if neccessary. So function call overhead can be ignored. Anything in the body of the function needs to be there to ensure the object remains in a valid state, so it will be performed whether or not you use a function to access the variable. So that overhead can be ignored. But you might forget to call those validation functions, which means you've now increased the possibility of additional bugs without recieving anything in return.

CM

Share this post


Link to post
Share on other sites
the correct choice to make is that one that will cause you less hassle. Usually this means making everything public. However, if you have an invariant that you want to maintain (the normalized vector was an excellent example), then you probably want to keep the data private, because some ignorant coworker (and that can be you, you are your own coworker!) might come along and screw things up.

So yeah, when in doubt make it public. It is the easier thing to do. Why spend your effort fixing problems that might not be there? We all make mistakes coding but we make different mistakes more often. So figure out which mistakes you make most often and focus on them. Me? I spell strings wrong. I once ruined a program that I had been working on trying to find a bug (oh how I wish that I had source control) that turned out to be that I loaded something like config.tx instead of config.txt! My brain almost exploded after I spend 30+ hours looking for it.

Share this post


Link to post
Share on other sites
Quote:
Original post by Glak
the correct choice to make is that one that will cause you less hassle.

So yeah, when in doubt make it public.
I'm not an expert c++ programmer, so anything I have to say on the subject can certainly be ignored. But I do have to say that I disagree with both of these statements. I don't know whether you mean hassle to code, or hassle to use, but in either case I don't know that avoiding hassle is a particularly good way to motivate software design decisions.

Also, I'd say when in doubt make it private. If you are going to make your variables public you should be sure that you'll never need to change the internal representation or implementation. Even the simple case of public xyz variables can backfire if, say, you later decide to templatize by dimension or otherwise take a more generic approach to your vector library. In this case it's not even about maintaining invariants; you can expose the variables through inline functions that return non-const references. What you're protecting is the implementation, which gives you the freedom to change or improve it without affecting existing code.

Just my $.02.

Share this post


Link to post
Share on other sites
I have heard these arguments about interface vs implementation many times, and they would be right, were they not based on an assumption that in my experience is usually false. Sorry about that being so long winded. I find that I change interfaces more than I do implementation.

I generally attempt projects that are barely within my ability to accomplish, and I think that most game programmers do the same. Maybe there is some pro out there cranking out his 10th version of tetris, but I think that realisticaly most of us are attempting what is for us a very big project. This means that I don't have time to go back and optimize. I'll save that for when I finish the project, except that I might just move onto a grander project, or I might not even finish the current project. So chances are the implementation won't ever change. The interface might change though, as I might have made a poor design decision.

Share this post


Link to post
Share on other sites
The point of interface vs implementation isn't necessarily that you need to compile less if you change the implementation, but that you specify a specific contract, expressed as pure virtual member functions, without having to put any implementation details in the contract.

In the implementation of the interfaces, you put the concrete class in a .cpp file, and make all your members public. This makes it really easy to unit test the class, and makes it easy to implement small, collaborating classes without having the befriend everything.

In my mind, there are three kinds of classes:

1) interface classes
2) concrete implementations
3) value classes

The "value class" is your typical Rect, Vertex, or Matrix. The difference between an interface (and its implementation), and a value class, is operator==(). If operator==() examines members and returns equality based on member values, then it's a value class. If it makes more sense to just do operator==() based on the pointer values, then it's an interface/concrete class.

I make the rule that value classes should not have virtual functions -- if you use virtual functions, chances are you want to overload the class, which means that a simple examination of members isn't usually a correct operator==().

Share this post


Link to post
Share on other sites
Thanks for all the replys. I think I understand why the cone3d tutorial has used a struct for the frame class and made everything in the base class public, its because the sprite class is the one that the user actually deals with really, it has all of the functions for the user in it. Once you have set up the base you don't need to touch it again directly. I suppose someone could mess about with the spritebase but I don't see how I could by mistake. Sound good? Thanks

Share this post


Link to post
Share on other sites
From my experience programmer who're new to OOP have difficulties with making all variables private and _don't_ add get&set-methods für them. But after some years of practice you'll write classes, that're using variables to store internal data and the public interface just don't need to give them out, because the class' user doesn't want to change that variable, he wants to use the class for the one thing it was written.

For example, take a look at the classes in the C++ standard library they've an interface that let you do everything you need (and more!) but they just don't need get/set-functions for their private variables, because they're only a detail of the implementation.

Share this post


Link to post
Share on other sites
True I suppose. I guess the best thing to do is to get lots of practice in (which I am going to making this game) so I'l leave it alone for now and maybe re-do the code when I know what I'm doing :)

Share this post


Link to post
Share on other sites
It depends, on what the variables and how it works.

First of all, is it safe to make it public? For a member to be public the class should expect any possible value for that member and work correctly with any possible value in that member.

Is that value independant? Some classes might not work correctly if you change a value to something without changing some other value. Also a new value could also mean that something else has happened.

Finally , encapsullation is one of the concepts of Object oriented programming, so people shouldn't have public class variables. But not everybody cares about that, in fact, I don't. And if a variable member works correctly being public then it would be a little speed performance and less members for the classes.

Share this post


Link to post
Share on other sites
I know a surprising number of people that use the speed reason as the justification to make a bunch more stuff public than should be. It's been a while but I seem to remember comparing the code from a public variable to the code generated from an inline accessor to a private variable and they produced the exact same machine code. Assuming this is still true(it should be), speed is a completely wrong justification for making things public(in most cases).

Share this post


Link to post
Share on other sites
Well it is not really a comparission of just the public and the private keywords

mostly when something is private you would still need a method for knowing its value. So it is a read only attribute.

Thus you would need a method that just returns the variable's value. and that's somehow slower than using the variable directly.

Although on p4 times that has stopped to be a worthy issue

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
Quote:
Original post by Vexorian
Well it is not really a comparission of just the public and the private keywords

mostly when something is private you would still need a method for knowing its value. So it is a read only attribute.

Thus you would need a method that just returns the variable's value. and that's somehow slower than using the variable directly.

Although on p4 times that has stopped to be a worthy issue

Today's compiler have an option called optimization, maybe you should turn that on.

Share this post


Link to post
Share on other sites
I'd say that there are no hard and fast rules for this sort of thing. Sometimes they make sense, sometimes they don't. But in general, what benefits are there for public members? A little less typing is about it - o.m_length = value instead of o.SetLength(value). If that even is less typing.

The lamest thing is when you are debugging and find yourself looking at a member variable on some object and you say to yourself "how the heck did that get in there?". If there are hundreds of places that m_length is assigned to, you've got a chore on your hand. There are ways to get the debugger to do some of the work but I find it a lot easier to set a breakpoint in the SetXXX function instead.

The thing in general to rememeber about encapsulation is that you arn't trying to protect against malice but against carelessness.

Share this post


Link to post
Share on other sites

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