C++ Inline get/set methods

Well, making the data public is the most straightforward way, and manipulations does not require a function call, so they CAN be faster (not the other way around that you wrote but probably didn't mean).

But, when compilers inline the get/set methods for private data members, there is neither the overhead of calling a function. Making get/set functions does allow you to do some checking, caching or other side stuff that must happen when a variable changes.

I am unsure though when the compiler makes a function inline...even if you say inline it may even not inline it! I wish it wasn't a hint, but an order. So, when do functions get inlined? When the compiler thinks it's better to inline?

It is unlikely to make a significant difference.

Quote:

Original post by antimouse ...the good programming practices of declaring things private...

This is not a general rule that produces good programming practice. Point, vector and matrix have no invalid values for their members, so therefore no checking of values is required. Good programming practice here IMHO is to make the members public.

If you need to constantly validate a member, consider instead of having get/set methods rather move the processing that requires access to these into class methods that describe actions rather than values.

Bad:

class Person{private:    int health;public:    int GetHealth() const { return health; }    void SetHealth(int v){ health=v; }}void f(Person &p){    int h=p.GetHealth();    h=h-2;    if(h<0) h=0;    p.SetHealth(h);}

Better:

class Person{private:    int health;public:    void Damage(int v){ health-=v; if(health<0) health=0; }};void f(Person &p){    p.Damage(2);}

Quote:

Original post by antimouse I always think about that because I have some templated classes for point, vector and matrix and I use inline operators and methods to access the data members. As in my applications speed is required, I always wondered if I should abandon the good programming practices of declaring things private in order to obtain better performance.

Wrong thing to worry about.

Look at it this way. If you need to set individual values one by one (x here, y and z there, x and y over there...) you are already using slowest possible way, one which will be marred by cache misses, branch mispredictions and other problems.


If however you need to blast thousands of coordinates each frame - a memcpy or simd will do wonders for performance. Or better yet, off-load that to GPU altogether.

The difference between two approaches we are talking here is 1000x.

Function call overhead and cost of stack allocation and speed fo for loops and such are things from days long gone. All the speed today comes from how well you can utilize the cache, and streaming data linearly is clear winner here.

Quote:

Original post by Aardvajk Quote: Original post by antimouse ...the good programming practices of declaring things private... This is not a general rule that produces good programming practice. Point, vector and matrix have no invalid values for their members, so therefore no checking of values is required. Good programming practice here IMHO is to make the members public.

I would still keep them private for few reasons:
- Easier to use IDE to find all accesses.
- Easier to set breakpoint.
- Ability to enable checking in debug build, for out of bounds, NaN's etc.

Point, vector and matrix are used so widely, any changes to the accesses would cause major headache.

For performace, I'd not inline them until the final touches before product ships, shown by profiler. I have seen simple getters jump in profiler to take like 1% of CPU. Usually this is fixed by not repeatedly dereferencing stuff or using getter in inner loops, and ultimately making them inline.

Making them inline prematurely prevents them showing in profiler.

I can't think of any situations in which you'd want to put a breakpoint on "all accesses of any Vector3's y member". More likely you'd put a data breakpoint on the specific one you're interested in, and that doesn't require getters/setters. As for catching out-of-range and NaN, the former doesn't exist in the math classes the OP was talking about, and the latter is better caught by enabling FP exceptions.

I was being a bit unclear. I was thinking more in line of get set functions generally, not specially for math classes.

But even for math classes, there isn't any disadvantage for making data private. I've experienced cases where 3rd party API return NaNs, but you're right the check for that doesnt belong in math classes. Changing matrix from row-major to column-major was suprisingly easy.

Quote:

Original post by Ftn But even for math classes, there isn't any disadvantage for making data private.

You're right that the stakes on the other side are pretty low, so this is basically just me indulging in some bikeshedding, but IMHO per-element getters and setters make vector stuff awfully wordy. v.y = -1; is cleaner and nicer than v.SetY(-1);. It looks more like what it is, namely, changing an element of a utility struct, rather than issuing a message to an encapsulated, self-sufficient capital-O Object.

First of all, thanks for all your replies so far.

I liked the answer by Aardvajk, it made a lot of sense to me. But then I keep thinking why some well-known libraries, like CGAL (used for Computational Geometry) use private data members for vectors and matrices.

And about what Sneftel said, I personally didn't use things like "v.SetY(-1)", I overloaded the [] operator for getting and setting, so it becomes a lot more clean "v[0] = -1" or "v[X] = -1" if I define some constants for the coordinates.

Quote:

I liked the answer by Aardvajk, it made a lot of sense to me. But then I keep thinking why some well-known libraries, like CGAL (used for Computational Geometry) use private data members for vectors and matrices.

I prefer private data for matrix classes because it allows you to abstract away how the data is managed and stored. For vectors, it matters less, IMO, although making the data private does make it easier to change the underlying representation if the need arises. (For vectors, my own preference is to make the data private and use the [] operator for element access.)

Quote:

Original post by antimouse And about what Sneftel said, I personally didn't use things like "v.SetY(-1)", I overloaded the [] operator for getting and setting, so it becomes a lot more clean "v[0] = -1" or "v[X] = -1" if I define some constants for the coordinates.

That gives you the worst of both worlds. Since you have to return a reference for that to work, you have no opportunity to set a breakpoint on changes (except in the case of returning a proxy setter, but that presents huge problems of its own, and in any case you're not doing it), and you can't perform any range or NaN testing. Meanwhile, making v[X] work forces you to define X as a global variable (or worse, a macro). Do you really want vectors to work differently (and wrongly) in functions or other namespaces where X is being used as a variable name?

Quote:

That gives you the worst of both worlds. Since you have to return a reference for that to work, you have no opportunity to set a breakpoint on changes (except in the case of returning a proxy setter, but that presents huge problems of its own, and in any case you're not doing it), and you can't perform any range or NaN testing.

Sorry, I didn't understand this part. What do you mean by "set a breakpoint on changes"? You mean breakpoints for debugging? It that's what you meant, I didn't get it.

Quote:

Meanwhile, making v[X] work forces you to define X as a global variable (or worse, a macro). Do you really want vectors to work differently (and wrongly) in functions or other namespaces where X is being used as a variable name?

It was just an example, but if I declare X as a constant inside a namespace, I can't see any problems with it, but maybe I'm wrong. I personally don't like using these constants, so I would rather stick with the numbers.

Another advantage of indexed access for vectors is that it makes it easier to implement loop-based algorithms, or to implement common geometrical algorithms in a more generic way. Also, it doesn't have to be either/or - you can offer support for both named and indexed element access in the same class, if that's what you prefer.

As for indexed access, I don't bother with (e.g.) v[X], mostly for the reason Sneftel mentioned (v[0] reads just fine to me).

Quote:

Original post by antimouse Quote: That gives you the worst of both worlds. Since you have to return a reference for that to work, you have no opportunity to set a breakpoint on changes (except in the case of returning a proxy setter, but that presents huge problems of its own, and in any case you're not doing it), and you can't perform any range or NaN testing. Sorry, I didn't understand this part. What do you mean by "set a breakpoint on changes"? You mean breakpoints for debugging? It that's what you meant, I didn't get it.

Well, yes, that's what a breakpoint is for. The advantage of SetX() is that you can make your breakpoint fire when someone calls SetX(), but not when they call GetX(). If you just do operator[], there's no way to know whether the user is setting or getting.

Quote:

It was just an example, but if I declare X as a constant inside a namespace, I can't see any problems with it, but maybe I'm wrong. I personally don't like using these constants, so I would rather stick with the numbers.

Well, which namespace? If you put it in Vector3's namespace, you need to do v[Vector3::X], which obviously is quite ugly and wordy. And if you put it in the user code namespace, it still means you can't reuse X for anything else, but it also means you have to manually put those constants in every namespace that uses Vector3, and keep them all up to date separately. That's.... pretty bad.

Personally, I much prefer accessing with x,y, etc than 0,1, etc. Given a quaternion, does 0,1,2,3 map to x,y,z,w, or does it map to w,x,y,z? I've seen it both ways. Give me human-readable names any day... that's why they're there. That doesn't mean you shouldn't also provide indexed access... as jyk pointed out, looping can be useful.

Um, if we're going to use setters/getters...

class Vec2{    int x_;    int y_;    public:    //overloading FTW!    int x(){return x_;}    void x(int new_x){x_ = new_x;} //in other classes, you might do range-checks or whatever    int y(){return y_;}    void y(int new_y;){y_ = new_y;}};

I don't think implicitly callable functions made it into the C++0x standard. That would have been nice.

Quote:

Original post by theOcelot Um, if we're going to use setters/getters... class Vec2{ int x_; int y_; public: //overloading FTW! int x(){return x_;} void x(int new_x){x_ = new_x;} //in other classes, you might do range-checks or whatever int y(){return y_;} void y(int new_y;){y_ = new_y;}}; I don't think implicitly callable functions made it into the C++0x standard. That would have been nice.

Yes, this is about the nicest method I find to use. No extra name clutter, pretty straight forward to use. Implicit callable functions would indeed be rather nice from a design point of view! But It also makes it harder to track all the implicit stuff that can be called by one statement :P, I find it can get difficult when you're on the edge of what's possible ^^

The indexed/named discussion above got me thinking of a thread I ran into long ago:

http://www.gamedev.net/community/forums/topic.asp?topic_id=261920

Best of both worlds.