Sign in to follow this  

Getters and Setters (C++)

This topic is 4332 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

Is it always good practice to prefix the names of accessor methods with "get" and "set"? For me, this seems a bit cumbersome, and I have already removed "get" from most of my classes. Would it be a good idea to remove "set" as well, or would this cause confusion? Ruby seems to have done away with "get" and "set" by defining attributes that blur the distinction between methods and members. What naming methods do you prefer? Examples: class Foo { public: int getBar() { return m_bar; } void setBar(int b) { m_bar = b; } private: int m_bar; } or class Foo { public: int bar() { return m_bar; } void bar(int b) { m_bar = b; } private: int m_bar; }

Share this post


Link to post
Share on other sites
Although I realize these are just simple examples, your post is probably going to elicit some comments on the point of encapsulation :-) Although I don't know that it's necessarily always bad to have get and set functions (it does allow you to validate input, change the internal representation, etc.), it's sometimes an indication that you could rethink your design a little.

Share this post


Link to post
Share on other sites
You don't really need to prefix your accessor functions with 'get' and 'set'; but, it does make the intentions of those functions clearer to user. If you're the only person to ever use that class, then it's ultimately your call; however, if anyone else were to use your code, you would have to clearly document the usage of those functions, otherwise, people won't have the foggiest clue as to what they do.

So, in essence: yes, it's good practice to prefix accessor functions with 'get' and 'set', if nothing else other than to make the usage of the function clear.

Share this post


Link to post
Share on other sites
I prefer to prefix setters and getters with Set and Get, just like in your first example. In your second example, there's no hint to the programmer of what both bar methods are doing, and it's going to be irritating if you'll be using them a lot. Consider this example:



{
MyWonderfullClass obj;
obj.stuff();
}



Can you tell me what is method MyWonderfullClass::stuff() doing without looking at its definition? Is it ordinary method, which you call when you want to achieve sth, ie. update object state, or is it getter, but somebody have forgot to save returned value somewhere?



{
MyWonderfullClass obj;
obj.GetStuff();
}



Now you know you have error here.


Of course, it's only IMHO.

Share this post


Link to post
Share on other sites
If you use VS maybe use "property"? Some people don't like it, some people use it, but it's upto you.


public:
_declspec(property(put = PutTest, get = GetTest)) int iTest;

void PutTest(int iTest)
{
this->m_iTest = iTest;
};

int GetTest(void)
{
return this->m_iTest;
};

private:
int m_iTest;





Then you can just use iTest.
Yes some people say it's not safe to use it because you can do = or something by mistake, but again it's upto you, if you know that you don't make mistake like that then use it!

Share this post


Link to post
Share on other sites
FWIW, the standard library does not put in set or get prefixes in the cases where those classes expose an accessor/mutator pair: consider for example std::stringstream::str(const std::string&).

As a general rule, it is not wrong to emulate the interfaces presented by the standard library. (The implementation techniques, on the other hand... ;) ) Of course, it's worth emphasizing that they present such pairs in rather few cases, unless you count things like container operator[] overloads - but then, containers are really different things from objects.

Share this post


Link to post
Share on other sites
First of all, it is generally very very rare to have both a getter and a setter. If you're finding that you have both and they just pass data through quite often, you should probably examine your design and see if they could be eliminated in some way.

In the case that Zahlman mentiones, the str() function (iirc) passes it's parameter to another function belonging to a member variable that is a template parameter, so the stream class itself has no idea what the data is being used for and is acting only as a 'middle man' to another class that should actually process the data. It's one of the rare cases where this type of design makes sense, because the stream class can't really say what is being done with the string since it doesn't process it.

In my personal code, I often have 'actions' and 'readers'. For example, in a work project I have a class that, at it's core, scrolls an image across the screen. One member function is 'GoToLine(AbsoluteLineNum)', which is more descriptive than 'SetCurrentLine' and goes well with other functions (named 'GoTo_(_Val)' with _ being the name of various types of data associated with each line). It also makes the existance of a 'Scroll(DeltaNumLines)' function logical, because IMO the action-based name ('GoToLine') says "this function does some work" and not "this function sets a variable", which leaves room for a "this function does similar but lesser work" function.
A name such as "SetCurrentLine" obscures the fact that some work is done and makes it sounds like all that happens is that a variable is set to the value you provide. If that is the case, there isn't much reason to have the scroll function because doing a 'Set(Get+K)' should be just as good, right?

By 'readers', I mean essentially 'getters' minus the 'get'. In the above example, the functions to retrieve the extents of the image displayed would be 'TopLine()' and 'BottomLine()'.

It took a while for me to get to that point, though, because my internship started the summer after my first year in college, and I didn't have any experience doing design of any kind. Even though my first attempts weren't exactly like that, they were very close and it ended up saving me, because later on more requirements were added to the class and it only too very slight changes to the code to support them due to the way things were encapsulated. The code using this class had to be changed also, but for the most part it was changes on the level of a few regex search-replace and the few other changes took literally a few minutes to make.

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
Quote:
Original post by Koshmaar
I prefer to prefix setters and getters with Set and Get, just like in your first example. In your second example, there's no hint to the programmer of what both bar methods are doing, and it's going to be irritating if you'll be using them a lot. Consider this example:


*** Source Snippet Removed ***


The source snippet is not good, as you will always be assigning from a getter, and passing in a variable to a setter, so the usage will always make it clear what the intention is. I stopped using 'get' and 'set' in front of the declarations after I saw someone like David Abrahams (or someone else considered to be of 'high stature' - I cannot find the post) say that it was idiotic to do so. It actually made it a little easier for me to code, as my brain did not have to parse the redundancy any more. The code 'looks' a lot cleaner, as well.

Share this post


Link to post
Share on other sites
Quote:
Original post by Koshmaar
Can you tell me what is method MyWonderfullClass::stuff() doing without looking at its definition?


No, because "MyWonderfullClass" and "stuff" are horrible, undescriptive names, and fail to comply to ANY definition of the term "self-documenting code".

Quote:
*** Source Snippet using "GetStuff" Removed ***

Now you know you have error here.


No I don't. For all I know, MyWonderfullClass is an HTTP request cacher, and "GetStuff" caches some commonly used resources locally. Some sort of caching or loading is what first springs to mind with such braindead names as supplied, not "I return a member variable, I'm so special!!!111". At least with such examples as provided!!!

"So it's a contrived example - my point still stands" you say?

No. Most of the time, it's clear what a function does based on the bits you omitted and replaced with junk (MyWonderfullClass, and stuff). When it isn't, the fact that the function has a wonderful "Get" prefix is very very unlikely to help, and you'll much rather wish your programmer had spend those three keystrokes typing something with meaning - describing what the function does - rather than simply tell you it's returning a member variable, which at this rate, you're going to find out when you open the source yourself to apply the wonderful concept of nouns to the class and function names.

At least, that's what I'd be doing right now if I was working on a project with that class/function in it - both to add nouns, and to find out what the **** that function/class does (with or without the "Get" prefix). Because both are necessary if I'm going to maintain sanity working on that codebase.

-Mike

[Edited by - MaulingMonkey on January 29, 2006 2:16:43 AM]

Share this post


Link to post
Share on other sites
Quote:
Original post by Anonymous Poster
The source snippet is not good, as you will always be assigning from a getter, and passing in a variable to a setter, so the usage will always make it clear what the intention is. I stopped using 'get' and 'set' in front of the declarations after I saw someone like David Abrahams (or someone else considered to be of 'high stature' - I cannot find the post) say that it was idiotic to do so. It actually made it a little easier for me to code, as my brain did not have to parse the redundancy any more. The code 'looks' a lot cleaner, as well.


This is a good point, and it is why I am considering removing the "set" prefix from my code as well. In ruby, getters and setters look exactly like public access (var = Foo.bar, Foo.bar = 17). If there is no confusion there, then I don't see how there can be confusion with getters and setters minus the somewhat redundant prefixes.

Share this post


Link to post
Share on other sites
sometimes I just overload the get/set functions if Im using them. a void set function, and a get function that returns the type. But thats because in my head, having a function thats like window->height() sort of implies that im getting a value, not setting one. Whereas, doing something like window->height(600) to me, implies that Im setting a value. But other people might not read it like that in their head.

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
Quote:
Original post by Mr Awesome
This is a good point, and it is why I am considering removing the "set" prefix from my code as well. In ruby, getters and setters look exactly like public access (var = Foo.bar, Foo.bar = 17). If there is no confusion there, then I don't see how there can be confusion with getters and setters minus the somewhat redundant prefixes.
Umm... So Ruby's properties lead to no confusion because their usage looks exactly like public access. But in C++ if you drop the prefixes, the getters and setters still *don't* look like public access. I can't see how you don't see that this difference could lead to confusion.

Share this post


Link to post
Share on other sites
I prefer to prefix Get and Set. I don't get how the code really looks "cleaner" by omitting them. Shorter, yes, easier to type, yes, but unless the user knows specifically that TacoBell::SauceFormula() is both the get and set, it's going to confuse the hell out of the person using it.

To each his own, however.

Share this post


Link to post
Share on other sites
Quote:
Original post by Anonymous Poster
Quote:
Original post by Mr Awesome
This is a good point, and it is why I am considering removing the "set" prefix from my code as well. In ruby, getters and setters look exactly like public access (var = Foo.bar, Foo.bar = 17). If there is no confusion there, then I don't see how there can be confusion with getters and setters minus the somewhat redundant prefixes.
Umm... So Ruby's properties lead to no confusion because their usage looks exactly like public access. But in C++ if you drop the prefixes, the getters and setters still *don't* look like public access. I can't see how you don't see that this difference could lead to confusion.


They don't look exactly like public access, but the syntax is consistant without the prefixes, and wouldn't lead to being any more confusing.

You have:


public access: set: foo.bar = baz get: baz = foo.bar
C++ prefixless get/set: set: foo.bar( baz ) get: baz = foo.bar()
ruby explicit(?) call: set: foo.bar=( baz ) get: baz = foo.bar()
prefixed get/set: set: foo.set_bar( baz ) get: baz = foo.get_bar()


I fail to see how any of these could be ambiguous, and confused with one another (get/set that is). Which is the only reason that I can see cause for confusion that set/get prefixes would solve anyways, which is our current topic.

Quote:
Original post by Flimflam
Shorter, yes, easier to type, yes, but unless the user knows specifically that TacoBell::SauceFormula() is both the get and set, it's going to confuse the hell out of the person using it.


I've never been confused by this kind of thing, since it's usually pretty obvious. Then again, I've never used such silly variable names in production code. Similarly, I've never been confused by Get/Set prefixes, since it's usually pretty obvious. Then again, I've never found them of much help, either.

Share this post


Link to post
Share on other sites
See, Ruby doesn't use 'get' and 'set' prefixes because it's capable of working with properties natively. With C++, more specifically unmanaged C++, you don't have native access to property types. Now, if you were using managed C++, you might be able to get away with it. I know you can use properties in C#, so you wouldn't need the two prefixes in that case.

That being said, however, it would come down to the usage of the function. In the case of Extrarius, those functions performed actions, so it made more sense to use a function name with a verb. But, not all cases will be an action. Take this for example:

You have a class that encapsulates the properties of an employee working for you. This employee has a name, and just got married, so she changed her last name. If you were to drop the 'get' and 'set' prefixes from the accessor/mutator functions, those functions would now look quite ambiguous.


class Employee
{
public:
void firstName(const std::string& sFirstName);
std::string firstName();
void lastName(const std::string& sLastName);
std::string lastName();

protected:
std::string m_sFirstName;
std::string m_sLastName;
};



Now, to you, this wouldn't make much of a problem, since you're the one that's coded the class; however, think of someone with a little less experience than you, that has come into the project months later. If that person is used to using the prefixes in their code, they'd get confused by this.

Of course, this isn't the best example, since, if you know anything about accessors and mutators, this code will become immediately clear; but, that's just my point. You need to be aware of the people that will ultimately use your code. If it's just you, and nobody else will ever use your code, then use your best judgement. On the other hand, if there is a chance that someone else is going to use your code, you need to be aware of their experience, or lack thereof.

In my case, the library I'm designing for my college course is being developed for new programmers in mind. In this case, I would leave the 'get' and 'set' prefixes in to reinforce the usage of accessors and mutators.

Share this post


Link to post
Share on other sites
Quote:
Original post by Dorvo
Now, to you, this wouldn't make much of a problem, since you're the one that's coded the class; however, think of someone with a little less experience than you, that has come into the project months later. If that person is used to using the prefixes in their code, they'd get confused by this.


For about 5 seconds? Of course, if you switch to using Get/Set, they'll instead be confused by the Standard Library (iostream's rdbuf, std::stringstream::str, the numerous functions not prefixed with get/set everywhere). IMO, Get/Set tuples are over-emphisized anyways, over picking class-wide operations instead of focusing on individual members - or acknowledging a certain structure as plain old data - would be a better use of time. Plus, it's more consistant - and thus easy to remember. Consistant capitalization, verb emphisis, and so forth.

Share this post


Link to post
Share on other sites
I don't understand why people think 'properties'(magic syntax to make what is apparently variable access act like function calls). Why should you hide what is really going on? If you want a public member variable, why not just make a public member variable?

Getters and Setters almost always break encapsulation and abstraction by saying "I'm a lazy object, so you'll have to update my state for me", which is against the principles of the OO paradigm.

The idea behind OO is that you(the programmer) tell objects to perform some action, and the object updates it's state to reflect that action (or returns failure, etc).
'Getters' make sense in a way because they provide a way to inspect the current state of an object, but care must be taken to avoid coupling the externally visible state and the implementation's representation.

For example, the image class mentioned in my previous post had a getter 'GetCurrentLine()'(instead of 'TopLine()' and 'BottomLine()') because it was originally designed to scroll automatically.The 'Current Line' was the last line it drew, and thus depended on the scroll direction - scrolling up meant new lines are appended to the bottom, and thus Current Line was on the bottom. This was not only confusing but also fragile. As the system was coming together, it was decided it would be to have the application to manually call a 'Scroll(DeltaNumLines)' function when it wanted to scroll, and in that context a 'CurrentLine' made less sense than it did before (which was admittedly very little). By making the accessors more generic (Top and Bottom line in addition to widget and image dimensions), the interface has been able to remain stable over many changes (including complete rewrites).

In general 'Setters' are a bad idea because you're eliminating abstraction. It's one thing to say 'ImageScroller.Scroll()' and quite another to say 'ImageScroller.SetLine(ImageScroller.GetLine() + ScrollSpeed); ImageScroller.Redraw();'. It might make sense to require the latter in some cases, but in the example I've given there isn't any reason for that - by definition it is the responsibility of the 'ImageScroller' to perform that function.

If you're creating classes like the one Dorvo listed, you should rethink your design. While an 'employee' could be called an object, the example is not an object in the usual OO sense because it's (conceptually) missing operations that manipulte the data. You might as well make 'employee' a struct with all-public members and no functions at all, because it's nothing more than a data record.
If you're not going to do any processing, there isn't a lot of reason to require a function call. Yes, planning ahead is good, but planning too far ahead for the indefinite and improbable future just obfuscates what's really going on.

Share this post


Link to post
Share on other sites
Of course, if you already have public members, and discover you need to insert code when they are gotten/set, you can do it without changine client code.


class HasProperty
{
private:
int pm_int;
public:

struct Property
{
int* data;
Property(int* data) : data(data) {}
operator int()
{
std::cout << "int gotten\n";
return *data;
}
int operator=(int rhs)
{
std::cout << "int set\n";
*data = rhs;
return *data;
}
};

Property m_int;

HasProperty() : m_int(&pm_int) {}
};

HasProperty hp;
int x = hp.m_int;
hp.m_int = x;



Notice that the get behavior doesn't trigger unless you assign it to something.

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
Quote:
Original post by Extrarius
I don't understand why people think 'properties'(magic syntax to make what is apparently variable access act like function calls). Why should you hide what is really going on? If you want a public member variable, why not just make a public member variable?
Because by using properties, you can change access functionality without affecting the code that uses the class. For example Dorvo's person-class could be changed to a remote access class with receives its data the first time you call a getter from some remote server, and perhaps stores the data to a local cache for faster future access. Of course, if you know that you'll never need such functionality from the class, you can just use public member variables (e.g. a 3d vector class).

Share this post


Link to post
Share on other sites
In my code I use get and setters, but only to allow inter class communication where you need to preinitialize some class internal parameters before you go on and use the object's memberfunctions

here is an example of a class I am currently working on which will be used with the singleton design pattern:
Some of you might know the Radiant editors, this class implements the selection behaviour of the radiant editors.

as you see most of the setter functionality is encapsulated in actions:
- drag to initialize a new selection box on lbutton click
- resize to resize the box on mouse motion
- deactivate to get rid of the current selection

the only setter is setAxis:
since around 4 2d views or 3 2d views and 1 3d view need to access the selection_box instance I need to setup the current axis:(top x=0,y=1; right x=1,y=2...)

the rest of my code is structured in the same way and I think this is quite easy to understand for third party programmers that might use this code somewhen in the future


namespace uni3d
{
class selection_box
{
public:
selection_box();
bool is_active() const { return m_bActive;};
void deactivate();
void setAxis(int32 x, int32 y, int32 z);
void drag(const cross::vector3d& pick);
void resize(const cross::vector3d& dir);
void setcolor(const float32 *pflColor);
void render();
private:
void copy(cross::vector3d& a,const cross::vector3d& b);
int32 m_iX;
int32 m_iY;
int32 m_iZ;
int32 m_iCorner;
int32 m_iPivot;
float32 m_flColor[3];
bool m_bActive;
cross::bbox m_OldBox;
cross::bbox m_Box;
};

typedef cross::singleton<selection_box> SelectionBox;
}

Share this post


Link to post
Share on other sites
I just write functions with decent names. If it's an accessor, it's prefixed by get, for example: getSceneNode(), or getVertexBuffer(). I've also used create, rotate, translate, render, calculate, eg: calculateNormals(), createTexture(), etc. Basically, the first lowercase word tells us what's going to happen, and anything afterwards is basically there for clarification (if there at all, on a scene-graph node, there's no point writing anything other than translate() ). Mutators (or the set variety), use set. They're pretty rare, actually. It's a loose system, there's no plan to it, but it does tell me what's going on.

Share this post


Link to post
Share on other sites
For me, I try to avoid the get/set mentality except where its truly needed. When I do use that methodology, I have come to do away with the Get/Set prefix. Its appearant to me that something like Bitmap.Height() returns a value, and Bitmap.Height(480) sets a value. That said, this is a poor example of where I might use the 'setter' version, because I'd likely, and do, use something like Bitmap.Resize(640, 480) to alter the dimensions of the bitmap, with Bitmap.Height() and Bitmap.Width() to read the current values.

Share this post


Link to post
Share on other sites
Sorry, I was busy for a moment (yup, exams).


Quote:
Original post by MaulingMonkey
No, because "MyWonderfullClass" and "stuff" are horrible, undescriptive names, and fail to comply to ANY definition of the term "self-documenting code".


"MyWonderfullClass" and "stuff" were meant to be horrible, undescriptive and failing to comply to ANY definition of the term "self-documenting names, since that was general example. I used general names, that were carefully designed to simulate a case, where you see piece of code, and have literally no idea of what it's doing. When programmer is writing his code in consistent way, then it's going to be easier to deduce what such piece is doing. When Get and Set prefixes are applied only to regular accessor methods, then it's very easy to tell what GetXXX() and SetXXX() are doing - they retrieve or set value of some XXX member value. However, when you see only XXX(), then in most cases, you have no idea about what XXX is doing - is is setter? or getter? or is it more complicated function, which does sth more sophisticated?

You know, looking to the documentation every time you see such function, can be a little irritating... ;-)

Quote:

No I don't. For all I know, MyWonderfullClass is an HTTP request cacher, and "GetStuff" caches some commonly used resources locally. Some sort of caching or loading is what first springs to mind with such braindead names as supplied, not "I return a member variable, I'm so special!!!111". At least with such examples as provided!!!


If one is using Getters and Setters uniformly in code, then he won't have problems with distinguishing functions which return values, from functions that ie. load/acquire resources. It's just a matter of sticking to some coding/naming rules.

Quote:
"So it's a contrived example - my point still stands" you say?

No. Most of the time, it's clear what a function does based on the bits you omitted and replaced with junk (MyWonderfullClass, and stuff). When it isn't, the fact that the function has a wonderful "Get" prefix is very very unlikely to help, and you'll much rather wish your programmer had spend those three keystrokes typing something with meaning - describing what the function does - rather than simply tell you it's returning a member variable, which at this rate, you're going to find out when you open the source yourself to apply the wonderful concept of nouns to the class and function names.


MaulingMonkey, you hate hungarian notation, don't you? [grin]

Well, to be honest, me too - I don't like it eaither... However, there's one little thing, which makes hungarian notation moderately (from time to time) useful, and it's been explained by Joel Spolsky in this article. Basically, it's all about making wrong code look wrong - IMHO that's very usefull concept. Beware, that I'm not the advocate of prefixing integers with i etc. - no no no.

IMHO prefixing function names with Getters and Setters falls within the same category, and people can argue using using similiar arguments.

I've mentioned this article, because I believe, that prefixing function names wihth Set and Get makes their behaviour completely clear to the reader. It's all about choosing clever function and variable names.


// probably I'll be eaten alive for writing 3 last paragraphs ;-)

Quote:
Original post by FlimFlam
I prefer to prefix Get and Set. I don't get how the code really looks "cleaner" by omitting them. Shorter, yes, easier to type, yes, but unless the user knows specifically that TacoBell::SauceFormula() is both the get and set, it's going to confuse the hell out of the person using it.

To each his own, however.


Amen, brother ;-) all in all, it's just a naming convention.

Share this post


Link to post
Share on other sites

This topic is 4332 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