Getters and Setters (C++)
Author
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;
}
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.
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.
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.
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:
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?
Now you know you have error here.
Of course, it's only IMHO.
{ 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.
If you use VS maybe use "property"? Some people don't like it, some people use it, but it's upto you.
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!
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!
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.
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.
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.
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.
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.
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]
Author
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.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
