Accessors and mutators are necessary?
Author
I'm writing a 2-d RPG, and I was going through the code of my Player class to add a field to it. I'm following the approach taught to me in school (private data member, public "set" and "get" functions), and I'm finding it a lot harder. Today I added a field called "turnsUntilAction", and it took me about 10 minutes when I had to put the "set" and "get" functions also. Moreover, I'll have easily around 15 members for each player (max HP/MP, current HP/MP, level, name, attack, defense, ...), and each pair of set and get methods adds *minimum* 10 lines of code (2 lines for the function prototype in the header file, minimum 4 lines in the class file for each function, plus 2 whitespace lines for each function.
This game is being worked on by me only (I have friends helping me out, but nobody else will be touching any actual code), and I don't plan on reusing the engine to make other games. Is there some reason that I'm not thinking of that makes it important for me to proceed with the "set"/"get" functions, or is it safe for me to convert my private members to public data members for my personal convenience?
Thanks! :)
While set/get-ers are not really necissary [and generally frowned upon], just making everything public is largely bad form as well. Generally, you want to make functions like 'damage' which does the proper effect to the proper stat.
To expand on what Telastyn said, you should probably have methods like this rather then get/sets:
You should still provide get functions to query the players state though.
//where health_t is whatever type you're using to store the current healthvoid player::damage(health_t amount){ if(currentHP <= amount) { currentHP = 0; deadFlag = true; } else { currentHP -= amount; }}void player::heal(health_t amount){ if(!deadFlag) { currentHP = std::min(currentHP+amount,maxHP); }}You should still provide get functions to query the players state though.
So basically, pass all values once in a constructor (or maybe a large "setEverything" method) and then whenever I need to change stats, make a function that changes the values in the proper context?
Thanks for showing me the damage/heal functions.. I like them! :)
Thanks for showing me the damage/heal functions.. I like them! :)
Even assembly language presents a higher level of abstraction than that monstrousity. That is quite possibly the worst advice I've ever seen in a thread about getters and setters. Congratulations.
Quote:Original post by Aiursrage2k
I say make your data members an array, then you pass it a constant which is the index.
...and abandon all yer (type)safety on the way.
Quote:Original post by ChocoArcher
So basically, pass all values once in a constructor (or maybe a large "setEverything" method) and then whenever I need to change stats, make a function that changes the values in the proper context?
Yes, I'd say that is the proper way. You just got one of the most important concepts of OO right: Encapsulation! [wink]
My general rule of thumb: Is there any combination of variable settings (for the class) that is invalid?
If yes, chances are your variables should be private. Examples:
std::vector - m_size should allways be equal to the number of elements newed.
some player/monster settings - negative hp may be disallowed as may reaching 0 or less without having a dead flag set.
a normalized 3d vector - should never be non-normalized (instead of get/set x/y/z I'd have get/set vector, which I would write using the conversion and assignment operators)
If no, chances are your variables should be public. Examples:
a simple 3d vector - any combination of x/y/z is valid.
player roster information - assuming any name + email + age combination should be valid.
That said there are exceptions to the no track: it can often be good practice to centralize certain aspects of variable modification. Let's take a player/monster case where any variable set is valid (negative hp simply means it is dead, no seperate flag mantained).
Damaging the player (can be) a complicated job, factoring in resistances to elements and the like. For single attacks it may be simple enough that such code is distributed all around the program whenever the player is attacked.
However, this is brittle, we cannot simply add a new stat for, say, "armor-resistance" (which we'll assume helps block everything) and have all of our 1000 equasions be updated to use this - we'd have to modify our 1000 equasions. This is bad, an example of tight coupling. The way to fix this problem is to instead use loose coupling. By having everything damage the player/monster/object through a damage() function, we can simply update the equasion in that one place and have the effect occur for all the 1000s of times that damage() function is called. Making the health property private helps corral users into using damage(), enforcing loose coupling.
If yes, chances are your variables should be private. Examples:
std::vector - m_size should allways be equal to the number of elements newed.
some player/monster settings - negative hp may be disallowed as may reaching 0 or less without having a dead flag set.
a normalized 3d vector - should never be non-normalized (instead of get/set x/y/z I'd have get/set vector, which I would write using the conversion and assignment operators)
If no, chances are your variables should be public. Examples:
a simple 3d vector - any combination of x/y/z is valid.
player roster information - assuming any name + email + age combination should be valid.
That said there are exceptions to the no track: it can often be good practice to centralize certain aspects of variable modification. Let's take a player/monster case where any variable set is valid (negative hp simply means it is dead, no seperate flag mantained).
Damaging the player (can be) a complicated job, factoring in resistances to elements and the like. For single attacks it may be simple enough that such code is distributed all around the program whenever the player is attacked.
However, this is brittle, we cannot simply add a new stat for, say, "armor-resistance" (which we'll assume helps block everything) and have all of our 1000 equasions be updated to use this - we'd have to modify our 1000 equasions. This is bad, an example of tight coupling. The way to fix this problem is to instead use loose coupling. By having everything damage the player/monster/object through a damage() function, we can simply update the equasion in that one place and have the effect occur for all the 1000s of times that damage() function is called. Making the health property private helps corral users into using damage(), enforcing loose coupling.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
