Scott Meyers Effective C++ 3rd Ed. Item 22 Declare members private
Pages: 12
GetHealth and SetHealth are bad. Monsters don't just walk up to you and perform damage calculations for you and then suddenly your health is a different number. Monsters also don't need to know your health.
GetName but not SetName is better. Someone else needs to know your name, so they ask you.
SetPosition but not GetPosition is better. The server decides where you are, and tells you. It doesn't ask you where you are, because it already knows and it doesn't trust you.
Whether or not getters and setters are bad is contextual, they're not all-around bad, just commonly misused, and it is easy to simplify an explanation and say they're bad.
In general, I follow:
-Setters should only be available to those in charge of you (e.g. a server)
-Getters should only be available to those who need to know (e.g. the UI)
-Getters and Setters should always be available to deriving classes who need to customize functionality
-Getters and Setters should be designed in a way that makes them as flexible as possible as to what implementation is used (e.g. storing a string, asking a lib, sending a network request, opening a file...)
And, they din't even have to 'look' like getters and setters. They also don't have to have 'get' or 'set' in their name:
But the name is your choice.
GetName but not SetName is better. Someone else needs to know your name, so they ask you.
SetPosition but not GetPosition is better. The server decides where you are, and tells you. It doesn't ask you where you are, because it already knows and it doesn't trust you.
Whether or not getters and setters are bad is contextual, they're not all-around bad, just commonly misused, and it is easy to simplify an explanation and say they're bad.
In general, I follow:
-Setters should only be available to those in charge of you (e.g. a server)
-Getters should only be available to those who need to know (e.g. the UI)
-Getters and Setters should always be available to deriving classes who need to customize functionality
-Getters and Setters should be designed in a way that makes them as flexible as possible as to what implementation is used (e.g. storing a string, asking a lib, sending a network request, opening a file...)
And, they din't even have to 'look' like getters and setters. They also don't have to have 'get' or 'set' in their name:
|
|
Last edited on
| L B wrote: |
|---|
| GetHealth and SetHealth are bad. GetName and SetName are better. |
Why is one bad and the other not?
I avoid naming accessors like that when I can. I prefer to give them verbal names so that they sound like actions rather than data access. For example, instead of a SetPosition method, my Character class has Move, which figures out the new position itself.
Sorry, I edited my post a lot - could you reread it and tell me if I answered your question?
@L B
Item 3: You are right - my bad.
Item 4: I am agreeing with chrisname. I meant in respect to the accessor solution, not chrisname's
Item 5: I just didn't see the point in having an extra function call versus direct access. I am starting to warm to this idea though.
Scott Meyers also talked about "future proofing" by making data members classes with their own interface, that way the internals can change without affecting everything else. I mentioned that much earlier.
I need to ruminate on all this a bit more - just need some sleep first ........
Item 3: You are right - my bad.
Item 4: I am agreeing with chrisname. I meant in respect to the accessor solution, not chrisname's
Item 5: I just didn't see the point in having an extra function call versus direct access. I am starting to warm to this idea though.
Scott Meyers also talked about "future proofing" by making data members classes with their own interface, that way the internals can change without affecting everything else. I mentioned that much earlier.
I need to ruminate on all this a bit more - just need some sleep first ........
Yeah, it's the "future proofing" that isn't possible with exposed protected data members. Unless the data members are instances of interfaces, in which case it's actually more useable than the getter/setter method, though obviously more work. I usually go for simple getter-setter because I don't develop large applications yet, though I will definitely be using the interface instance technique for larger things.
Last edited on
Oh right, I didn't see your edits. Yes, you did answer my question, and that's my reasoning as well. My Character class has DamageAttribute instead of SetAttribute; instead of taking a definite attribute value, it takes an offset and applies it to the attribute's current value. Similalry, Move takes a time and multiplies it with the object's current speed to figure out the offset, and then applies the offset to the position vector.
@chrisname yes, that is what I would do. What is your stance on protected data members now though?
> I don't know why everyone seems to have the mentality that getters and setters are bad.
http://www.javaworld.com/javaworld/jw-09-2003/jw-0905-toolbox.html they put the logic in the wrong place.
Make your getters and setters private.
http://www.javaworld.com/javaworld/jw-09-2003/jw-0905-toolbox.html they put the logic in the wrong place.
Make your getters and setters private.
@L B
I'm still not entirely convinced. I already keep implementation details private (for example, the GameObject class has a private, static Dictionary of Textures so that it never loads a Texture more than once) but some things IMO are worth exposing to the derived classes. C# has properties which are like accessor methods but transparent (they're code which pretends to be data), but I generally only use them when I need to do something with the value before getting/setting it. In C++ I do the same with accessor methods. I still don't think using accessors is worthwhile for everything.
@ne555
For me, it's because you don't go up to a dog and move its legs to make it walk. You call it over and it does the walking itself. Classes should do what they're told, and figure out how to do it by themselves.
I'm still not entirely convinced. I already keep implementation details private (for example, the GameObject class has a private, static Dictionary of Textures so that it never loads a Texture more than once) but some things IMO are worth exposing to the derived classes. C# has properties which are like accessor methods but transparent (they're code which pretends to be data), but I generally only use them when I need to do something with the value before getting/setting it. In C++ I do the same with accessor methods. I still don't think using accessors is worthwhile for everything.
@ne555
For me, it's because you don't go up to a dog and move its legs to make it walk. You call it over and it does the walking itself. Classes should do what they're told, and figure out how to do it by themselves.
Last edited on
@chrisname again you contradict yourself - you use protected data members but then you make classes manage themselves. I'm having trouble understanding how you can separate these concepts, perhaps you can make a code example I can understand?
It's different because one is to do with hiding stuff from the outside world, and the other is to do with hiding stuff from subclasses which, IMO, is unnecessary because the subclass can do what it wants with the superclass. I am coming around to the idea of being more secretive though.
Here's some of the code I've been talking about during this thread:
GameObject.cs: http://pastebin.com/CJkWBYC0
Character.cs: http://pastebin.com/bF4AFiMN
Player.cs: http://pastebin.com/GFs6eWdK
Bare in mind that the code is old (from August), unfinished and in dire need of refactoring.
Here's some of the code I've been talking about during this thread:
GameObject.cs: http://pastebin.com/CJkWBYC0
Character.cs: http://pastebin.com/bF4AFiMN
Player.cs: http://pastebin.com/GFs6eWdK
Bare in mind that the code is old (from August), unfinished and in dire need of refactoring.
| chrisname wrote: |
|---|
protected List <GameObject> Inventory; |
Well, it wouldn't matter because it's handled by the GameObject class anyway. I could make that private without breaking anything. Although I take your point.
Now that I have looked at this in the clear light of day - as opposed to going to the pub then making comments from 02:00AM onwards, things are making sense.
cire wrote:
OK thanks heaps, that makes a great deal of sense to me, and solves the problem I think.
Let's see if I understand it correctly now:
On the "Future Proofing" topic, with the CNutrition data member, we make it private and provide interface functions to it in the base class. Now it is like the "Template Method Design Pattern via the Non Virtual Interface (NVI) idiom", which Scott talks about in Item 35. The interface function setNutritionValues acts as a wrapper and can check (amongst other things) whether the user has sufficient authority to make changes. The real work is done by the CNutrition's member function.
@everyone - Do you have this book? It has a lot of really good stuff in it, and has certainly been thought provoking for me. I have learnt that although my protected data member approach worked for me, there are better ways of doing things.
Thank you very much to everyone who has contributed - I hope things are all going well at your end ! :D
I will probably mark this topic as being solved in the next day or so, but if any one has further comment then feel free to do so.
cire wrote:
| If the derived class needs to access the base class data, then the base class interface is not complete .... |
OK thanks heaps, that makes a great deal of sense to me, and solves the problem I think.
Let's see if I understand it correctly now:
On the "Future Proofing" topic, with the CNutrition data member, we make it private and provide interface functions to it in the base class. Now it is like the "Template Method Design Pattern via the Non Virtual Interface (NVI) idiom", which Scott talks about in Item 35. The interface function setNutritionValues acts as a wrapper and can check (amongst other things) whether the user has sufficient authority to make changes. The real work is done by the CNutrition's member function.
@everyone - Do you have this book? It has a lot of really good stuff in it, and has certainly been thought provoking for me. I have learnt that although my protected data member approach worked for me, there are better ways of doing things.
Thank you very much to everyone who has contributed - I hope things are all going well at your end ! :D
ThingsLearnt+=10;I will probably mark this topic as being solved in the next day or so, but if any one has further comment then feel free to do so.
Topic archived. No new replies allowed.
Pages: 12