getters & setters are evil - do I have the right idea?

[TheIdeasMan]

I have read some of the other threads about this subject, and the link

http://www.javaworld.com/javaworld/jw-09-2003/jw-0905-toolbox.html?page=1

provided by NwN, and this one

http://pragprog.com/articles/tell-dont-ask

provided by ne555.

I am wondering if I have interpreted these correctly with the following example, and I look fowrward to any comments you may have.

Say we have the following classes:

class CActor {
protected:
unsigned m_Health;
};
class Enemy: public CActor {
public:
void RecvAttack(CWeapon *pMyWeapon ) {//dont want to have Damage as an arg here
       m_Health -= pMyWeapon->m_Damage; //wont work - need a function to access
                                        //m_Damage - a get function??
     }
};
class CPlayer: public CActor {
public:
void Attack(CEnemy *pMyEnemy, CWeapon *pMyWeapon) {
      pMyEnemy->RecvAttack(pMyWeapon);
     }
};
class CWeapon {
protected:
unsigned m_Damage; //This is subtracted from Enemy's m_Health during an attack
public:
//need a get function here?
};

Already I maight be in trouble for designing class inheritance before thinking about realtime communication between objects. I understand from the articles above that I need to think about the communications between the player, enemy and weapon objects. I am thinking it doesn't matter that I might have 10 types of players, 50 types of weapons and 100 enemies all with their own inheritance trees - as long as I have interface between the 3 object types (player, enemy and weapon) done corrrectly.



So how to access the m_Damage variable in the CWeapon class without a public get function? Passing the damage variable as an argument doesn't seem to be a good idea, because what if in the future we changed damage & health to be double instead of unsigned? This would break the code as we would have to change every use of the getDamage function.

What I have tried to do is to have 2 functions - one on the player side and the other on the enemy side. I could have had 1 RecvAttack function that took a pointer to a Weapon, but that is the same problem - how to aceess the damage variable?

Even if the Attack function was an overloaded virtual function in the actor class (players can be attacked by enemies too) - we still have the same problem.

Now I have another question: If I have a class at or near the top of the inheritance tree (like the CActor class above), and I don't want to be able to create objects of that type. This is easy with a pure virtual function in that class, but what if I really don't want to have to redefine a function in all the derived classes? That is, I want the functions in the base class to be the same in the derived classes. I haven't tried, because I thought that it wasn't possible to have an ordinary virtual function in the base class with no implementation, then redefine it in the derived classes where necessary (hopefully once).

I look forward to your replies.

[Disch]

There are a ton of different ways to approach this problem.

The first question I would ask is... does CWeapon really need to be a class? How dynamic/encapsulated does it need to be? Are you planning on having randomly generated weapons of varying strength (like a roguelike) or are there a finite number of weapons with fixed stats?

In the case of a simple, finite group of fixed weapons, you might be better off with a struct and have all public members.

Not everything needs to be a class. If CWeapon's sole purpose is to group together a series of statistics, then make it a struct. That's what structs are for. Classes are for creating encapsulated objects.

[TheIdeasMan]

Hi Disch, Thanks for your quick reply.

I was asking this question for my own education and to help SGM3 in his thread:

http://www.cplusplus.com/forum/beginner/76885/3/#msg415888

As you can see there are all sorts of different weapons, all with their own values.

I can imagine a list of structs that stored all the values, but thought it would be more convenient to have them as classes, to make function calls easier, and to take advantage of the idea that a pointer to a derived class is a valid pointer to a base class.

virtual void Attack(CEnemy *, CWeapon *);

I could call this function with pointers to derived versions of CWeapon & CEnemy

[Disch]

As you can see there are all sorts of different weapons, all with their own values.

It looks to me like that's a misuse of inheritance. Why can't all those be combined into a single struct for simplicity?

But maybe I'm judging prematurely.

Inheritance makes sense when you want to have different behavior for different objects (ie: virtual functions). Are these weapon classes going to have anything other than getters/setters? Because if not you really shouldn't make such a complicated class hierarchy.

It really looks to me like CActor is doing all the work here. If you're passing the weapon to CActor and having it do the calculations, then what work does CWeapon do? Is its only purpose to be a container for weapon statistics? Because if so, that's a struct, not a class.

What are some functions in CWeapon and derived classes (other than getters/setters)? That might help me get a better grip on what you're trying to do here.

[TheIdeasMan]

Why can't all those be combined into a single struct for simplicity?

I was thinking they have different numbers of properties and different properties depending on what sort of weapon it is. A dagger would have less properties than a sword, and a Bow would have different properties than other types of weapons. Does this mean that you would have different structs to accomadate this? If so, wouldn't it be just as easy to have classes?

If there do have to be different structs to cover different groups of weapons, then I would have to overload the Attack function to cover each different type.

With the classes, I only need 2 Attack functions to cover player vs Enemy and Enemy vs Player:

virtual void Attack(CEnemy *, CWeapon *); //Player vs Enemy
virtual void Attack(CPlayer *, CEnemy *); // Enemy vs Player 

I am guessing there will be different behaviour for different weapons.

With the inheritance, there is inheritance happening - if I had a C structs then I would have to repeat variables that otherwise would have been inherited. If C++ structs we used, and they inherited from base structs, then I might as well have classes - A C++ struct is almost a class anyway.

It really looks to me like CActor is doing all the work here. If you're passing the weapon to CActor and having it do the calculations, then what work does CWeapon do?

What are some functions in CWeapon and derived classes (other than getters/setters)? That might help me get a better grip on what you're trying to do here.

You are right, the weapons don't seem to need many (maybe none) functions. I need to say that this is not my app - I am helping SGM3 as mentioned above. So I am guessing a bit myself as to how things are going to work, although I have a fair idea how I would do it.

Here is an example of what I have done in one of my Surveying apps - I created classes purely so that I would have different types for arguments to functions:

I had the distance classes CSlopeDist, CVertDist, CHorizDist; the Angle classes CBearing, CZenithAng. All of these classes have only one member variable - either a Distance or Angle respectively. So now I could have function calls:

CPoint3D* CalcNewPt(ExistPt, CBearing, CZenithAng, CSlopeDist)
CPoint3D* CalcNewPt(ExistPt, CBearing, CHorizDist, CVertDist)

I am wondering whether the situation with the weapons might be similar - Imagine a weapon belt ( a vector of weapons). It would be convenient to push back objects of type weapon.

I hope this has explained my thinking so far.

Going back to the original question - how to communcate between objects without getters & setters?

As for the other question, I should know by now to start a new thread for it.

I am very sure you have vastly more knowledge & experience than me - I am just somewhat of a basher compared to you, but I do try to think about what I am doing though. I would like to point out that I am not arguing with someone of expert status for the sake of it - I am just trying to understand. I appreciate your help - I hope I am not coming across as being stubborn clown !!

[mik2718]

How about this

void RecvAttack(CWeapon *pMyWeapon ) {//dont want to have Damage as an arg here
       m_Health -= pMyWeapon->damageCaused(); 
     }

?

yes, getter is bad, damageCaused() may not be an internal member of Weapon.

maybe you might want to create a new Weapon in the future where damageCaused is some sort of calculation - maybe random - whatever

[TheIdeasMan]

Hi mik2718, thanks for your reply.

damageCaused() may not be an internal member of Weapon.

I don't understand what you are saying here - wouldn't damageCaused() have to be a public member function that is effectively the same thing as getDamage() ?

I can see from reading the articles that get / set functions are bad - I am struggling to see how it might done without them.

Cheers

[mik2718]

Well its all in the name - what these articles are saying is that if all or most of your member functions are get/set then the class is not doing any work for you, and thats bad.

A couple of get/set in the interface is not too bad IMHO

The point of damageCaused is to try and abstract away from the idea of just outputting the member variable and towards a service that the Weapon class provides.

In may be the case that all your weapons will have an m_damage member variable so damageCaused will just be the same as getDamage

The idea of encapsulation is to set up your code so that in the future you can design and add new Weapons without changing a single line of existing code - they will just work.

The best object heirachy in C++ is just two levels, an abstract base class and concrete classes derived from it

class Weapon
{
public:
  int damageCaused() = 0;
  // other pure virtual functions
};

class Gun : public Weapon
{
public:
  int damageCaused()
    { return m_damage; }
};

class FlameThrower : public Weapon
{
public:
  int damageCaused()
    { return int(drand48()*m_damage); }
};

etc.

The main code uses only the public interface of the base class. Then when you derive a new Weapon class (whatever new weapon you have invented) then it just compiles and runs in your program without having to do anything else.

[TheIdeasMan]

@mik2718,

Thanks for your detailed reply.

All the things you are saying, are what I had in mind.

All the weapons classes will have a m_Damage, and I was going to have a getDamage function in the base class.

Maybe there is no getting around this problem, defining the interface properly in the base class could be sufficient.

However there is still the question of what happens if the type of m_Damage were to change to double say. Then I would have to change every line of code that calls getDamage . Maybe this is not so bad if the interfaces of the classes that need access to m_Damage, are set up in a similar fashion to the weapons class.

I wish the articles had provided some good examples.

I am still interested to see if anyone has any other thoughts.

Cheers

[Disch]

TheIdeasMan wrote:

I was thinking they have different numbers of properties and different properties depending on what sort of weapon it is. A dagger would have less properties than a sword, and a Bow would have different properties than other types of weapons.

This is another reason why it's a misuse of inheritance.

Inherited classes ideally should have the same interface as their parent class. Only the implementation should differ.

For example, you say that Bow has an additional property that Dagger does not have. How can you use that property if only given a CWeapon* pointer? You'd either have to downcast, defeating the entire point of having a shared parent.... or you'd have to give a dummy property to the Dagger so it can be accessed via the CWeapon interface.

Since the weapons themselves have no implementation, there's nothing to really distinguish them other than number of properties. Therefore a class hierarchy seems inappropriate to me.

Does this mean that you would have different structs to accomadate this?

No. Just have one. The struct will have entries to accommodate all weapons. For some weapons with fewer properties, those values will simply go unused.

With the classes, I only need 2 Attack functions to cover player vs Enemy and Enemy vs Player:

I'd think you'd only need one:

virtual void Attack(CActor*, const Weapon *);

Or do CPlayer and CEnemy not have a common base class?
Looking back at the thread you linked... I noticed that CActor doesn't exist. I could've swore I saw it there.

Anyway that would be a better example of inheritance. Players and Enemies do the same things (move, attack, get hurt, etc), but in slightly different ways. That is... the interface (what they do) is the same... but the implementation (how they do it) is different. That's what inheritance is for... and is where it truly shines.

But then again... I'm not sure why the object being attacked would need to know who attacked it. Ah well.... I guess that could be useful...

I am guessing there will be different behaviour for different weapons.

Then that's another matter entirely. If there's differing behavior then it would make sense to keep that behavior in the CWeapon class and provide a common interface by which that behavior can be triggered.

Though it's hard for me to explain that further without knowing details. I kind of wish SGM3 was involved in this thread so he could contribute and clarify the behavior he's looking for.

I had the distance classes CSlopeDist, CVertDist, CHorizDist; the Angle classes CBearing, CZenithAng. All of these classes have only one member variable - either a Distance or Angle respectively. So now I could have function calls:

That's a strong typing technique and can be very useful.

The question I have for you is did you inherit those all from a common base class? If so, did it provide any real benefit for you? If you're passing derived types around to different functions... what good does having a common parent do you?

After all, the purpose of having a common parent is to treat different types similarly... but you are intentionally doing the opposite there.

Going back to the original question - how to communcate between objects without getters & setters?

Establish an interface with which they can communicate. Getters and setters sort of work for that... but expose implementation details.



In the case where you want Weapons to have different behavior (ie: inheritance is justified) and want to use that behavior to impact another object (deal damage to an Actor)... then I'd approach that situation like this:

Determine the fundamental purpose of a weapon.

What does a weapon do:
- It inflicts damage by attacking.
- It might also offer some defense (ie: using a sword to block another sword)
- It might have some kind of stat-boosting enhancement
- It might have some magical ability (ie: holding a staff up to spit flames or something)

There are all areas that need an interface. Now... to my eye... the first 3 of these 4 points are simply statistics tracking and therefore do not warrant the use of a class hierarchy. But assuming that is not the case....


Design an interface that allows those purposes to be communicated to other objects. In the case of dealing damage... maybe that is just having a function return an int indicating how much damage is being dealt (it might look like a getter, but since it's implementation dependent, it might not always be one).

Or... if that's too simplistic for your needs you can wrap it up into a Damage struct. This would allow you do give elemental properties to an attack... or other status effects (ie, swords might induce bleeding, blowdarts might poison, etc).

That struct becomes the common interface. You can then pass that to the Actor and have it respond appropriately.


In the end you end up with something that seems similar to a getter (very much like mik2718's "damageCaused" function)... but for the purposes of this example we're assuming it's not a getter because you said weapons have unique behavior. (which kind of makes this a difficult example because how unique can the behavior of producing damage really be?)

I hope I am not coming across as being stubborn clown !!

You're not. Don't worry so much about offending people by asking questions ;)

This is a very complex topic and you'll hear tons of different opinions from different people. All I can do is offer my own.

[o3hC5Di1]

Thanks for your elaborate explanation Disch - this "hands on" example makes the other topic even clearer to me.

Now that you were talking about "the misuse of inheritance", the guy who published the "accessors are evil" article on javaworld also wrote "extends are evil": http://www.javaworld.com/javaworld/jw-08-2003/jw-0801-toolbox.html

He basically says that extends should be replaced by interfaces, with classes implementing those, in stead of extending a base class.

As far a s I know there's no actual interface keyword in c++, but making a pure virtual class would probably do the trick I would think?
So what's your opinion on that - is it another matter of "extends can be useful, but only when used appropriately" ?

Thanks again,

All the best,
NwN

[htirwin]

It seams like attack and receive maybe should be virtual functions declared in the base class. And maybe a CActor should have a weapon?

[ne555]

http://berniesumption.com/software/inheritance-is-evil-and-must-be-destroyed/ proposes to use composition (strategy).

void Attack(CEnemy *pMyEnemy, CWeapon *pMyWeapon) {
      pMyEnemy->RecvAttack(pMyWeapon);
     }

¿Why is this a method? It does not use/modify the state of the object.

If I have a class at or near the top of the inheritance tree (like the CActor class above), and I don't want to be able to create objects of that type.

¿why not?

[htirwin]

So how to access the m_Damage variable in the CWeapon class without a public get function? Passing the damage variable as an argument doesn't seem to be a good idea, because what if in the future we changed damage & health to be double instead of unsigned? This would break the code as we would have to change every use of the getDamage function.

I think that the main concern is making sure that others can use your classes and not worry about the details of the implantation. For example, in your game logic, you should not need to worry about the implantation. But communication between classes is part of the implementation.

I can't think of any way around either passing a specific type, or using a getter in this case, unless you wanted to do something as drastic as this:

#define HEALTH_TYPE double 

I would just pass it as an argument.

I'm not an expert though. I would be interested to hear more people opinions too.

[TheIdeasMan]

Hi Disch, thanks for your elaborate reply.

This is another reason why it's a misuse of inheritance.

Inherited classes ideally should have the same interface as their parent class. Only the implementation should differ.

My understanding of inheritance was that you define a base class with members that are common to all the derived classes below it, then define derived classes that have extra members than the base class, but they are still common to all it's derived classes. Actually, it could be done from the bottom, pushing variables & functions as high up the tree as possible. The power & elegance comes from functions that are only written once, then inherited.

If we are going have 1 struct with ALL the variables in it - is that not the same as a class with ALL the varibles in it? So a sword would have variables associated with Staves and Bows, which have nothing to do with swords (Apart from they are still weapons). I thought part of the idea behind inheritance was to avoid this. It wouldn't be very good if some of the classes had lots (dozens say) of member variables.

Since the weapons themselves have no implementation, there's nothing to really distinguish them other than number of properties.

I can see what you are saying here - if all a weapon does is inflict damage, it is easy to see how all the member variables could have static (const) values. I suspect there might be more to it than that - Eg Swords could have different actions that inflict differing amounts of damage.

With this type of app there are a myriad of behaviours and complexity that could be implemented. A good design should be able to be implemented with simple behaviour, with complexity added later on with out too much drama or extra work. I think it is definitely a good idea to start simple, get it working, then gradually add complexity.

For example, you say that Bow has an additional property that Dagger does not have. How can you use that property if only given a CWeapon* pointer? You'd either have to downcast, defeating the entire point of having a shared parent.... or you'd have to give a dummy property to the Dagger so it can be accessed via the CWeapon interface.

I guess the advantage of a pointer to a derived class being a valid pointer to a base class, without a virtual function, is really only handy for members they have in common. It saves having to overload the Attack function for every type of weapon.

It is handy for the damage member in the following way:

Say we have sword, staff and dagger weapons each with their own damage value. If we call the Attack function with a pointer to a particular weapon, it retrieves the right damage value because we gave it a pointer to that weapon.

To get at a member that is unique compared to the other weapons (like a member of CBow), I could have a virtual Use function that would do the right thing depending on which weapon pointer it was sent.

The question I have for you is did you inherit those all from a common base class? If so, did it provide any real benefit for you? If you're passing derived types around to different functions... what good does having a common parent do you?

Yes I did. There were more classes than what I mentioned above. One basic advantage was that the Radian value and it's get function was defined in the base class and inherited to all the others, so I didn't have to repeatedly define them for each class. There was more complexity also - Bearings are positive & only go upto 360 degrees, whereas angles can be +ve or -ve and be quite large (think of Sum internal angles of a polygon). There were also 2d & 3d angles.

After all, the purpose of having a common parent is to treat different types similarly... but you are intentionally doing the opposite there.

Not sure what you mean there. The classes were there purely to create different types, the only function was a get function. There was no class relationship between angles & distances.

I'd think you'd only need one:

virtual void Attack(CActor*, const Weapon *);

Or do CPlayer and CEnemy not have a common base class?
Looking back at the thread you linked... I noticed that CActor doesn't exist. I could've swore I saw it there.

CActor wasn't in the code in the other thread. I added it in this thread because Players & Enemies both have m_Health variables. You are right to put the Attack function in the CActor class - an example of pushing something as high up the tree as it can go.

I will have to ruminate a bit more on the rest of your thread thanks again for the great advice.

Cheers

[mik2718]

I used to do that, push all common stuff up the inheritance tree so as to share stuff amongst the derived classes. Then I realised I was wrong - push stuff the other way - down to the concrete classes.

I think the perfect inheritance tree has just two levels. An abstract base class with nothing in it but pure virtual functions.

Then a single level of concrete derived derived classes, no further derivation from concrete classes to "add extra functionality" - this just leads to complexity.

To make more complex stuff, use composition.

For sure this does not cover every case but I think you need good reasons to depart from it.

I think this accords with the links here from @NwN and @ne555

Maybe its an extreme view?

[TheIdeasMan]

NwN wrote:

As far a s I know there's no actual interface keyword in c++, but making a pure virtual class would probably do the trick I would think?

Yes, I read somewhere that these are the same things.

ne555 wrote:

¿Why is this a method? It does not use/modify the state of the object.

OK - fair call, an attack is really a ralationship between a weapon and an enemy.

TheIdeasMan wrote:

If I have a class at or near the top of the inheritance tree (like the CActor class above), and I don't want to be able to create objects of that type.

ne555 wrote:

¿why not?

Some of the classes are there to hold variables that are inherited to derived classes, they shouldn't represent objects because they are too general and don't represent a real object. Examples are CActor, CWeapon, CCutting. CCutting has variables which apply to it's derived classes like swords, daggers, axes etc. I don't want to, or shouldn't be allowed to create an object of type CCutting. For CActor, valid object would be of type CPlayer or CEnemy or derivatives of those. For CWeapon, we need a specific weapon like sword, axe, dagger, staff etc not just a general weapon.

I made a separate thread with this question, and it was answered by vlad.

Edit: I really need to go to bed - I've done an all nighter AGAIN.

[TheIdeasMan]

Here is the code from the other thread for easy reference.

class CWeapon {
string mName;   //almost forgot this one!!
float mDamage;  //maybe unsigned not sure how this works yet
float mAttackRating;  //a shorter name than this ?
float mAttackSpeed;
float mRange;
bool mCanCut;  //always true for CCutting - false for others
bool mSingleHand  //true for single handedness false for 2 handed weapons
bool mShort;  // Short or Long weapon - originally had this
                     // in CSword better here so can apply to clubs etc
};
class CCutting : public CWeapon {
float mCutEffective;   //cutting effectiveness - how well it cuts

};
class CSword  : public CCutting {
string mMaterial;  //assume this only affects swords should have enum of materials types

};
class CAxe :  public CCutting
class CDagger :  public CCutting;

//so far don't need individual sword types, as long as all the values can be determined 
//from the values in the base class variables

class CStaff :  public CWeapon;
class CClub :  public CWeapon;
class CBow : public CWeapon {
float mReloadTime;
unsigned mSpareArrows
};
class CNormalBow :  public CBow;
class CCrossBow :  public CBow;

So to summarise - my main question was answered by having well designed interfaces - that is fine by me.

The thing for me is that I thought I had learnt the how's & whys of class inheritance, now I find that the opposite idea is being suggested.

Even if it turns out that all the weapons have const damage values, and it is a simple model where damage is just subtracted from health, - I still don't see why one couldn't have inheritance. It would at least save on storage for unnecessary variables ( again one of the main ideas of inheritance). The weapons classes could have an interface via a virtual Use() function, that could return damage in the simple model, and do other more complex and specific things for a more complex model.

@mik2718

To make more complex stuff, use composition.

Just for my education - what does composition mean?

In your model with 2 levels of inheritance, how do you handle the CCutting class I had. Does the mCutEffective variable get repeatedly declared for Swords, Daggers & Axes etc?

Thanks for everyone's input.

[clanmjc]

Example Composition:

class CCutting{
   float mCutEffective;
//... impl
};

class CWeapon{
//... impl
};

class CAxe : public CWeapon{
   CCutting mCuttingAttributes;  //composition
};

VS.

class CCutting : public CWeapon{};
class CAxe : public CCutting{};

[ne555]

an attack is really a ralationship between a weapon and an enemy.

`magikarp' used `splash' against `charizard' it's supper effective
It's a relationship between characters, the weapon is part of the message.

If you are too worried about changing the representation of the health/damage, then you could follow iseeplusplus's suggestion. Or go further and make `healt_type' a class.

Take into account:
_ http://www.somethingofthatilk.com/index.php?id=135 (java)
_ the traditional way of cheating at golf is to lower your score.