class Player
{
private:
int str, dex, con, intel, will,
gold, exp, next_level, lvl,
threshold, atk, def, magic,
res, hits, m_hits, mana,
m_mana;
char name[55];
public:
void getStat(char[10]);
int setStat(char[10]);
char setName(char[55] cName) { name = cName; }
};
enum enmType { Archer, Dragon, Brigand, Bandit, Hellhound, Giant Tick, Daemon };
class Enemy : public Player
{
private:
enmType Race;
public:
enmType getRace(const enmType &);
void setRace(enmType cRace) { Race = cRace; }
};
Tell me if ive got this code right....
Author
Ok, ive got one class, Player, which stores stats and whatnot, then ive got a class, Enemy, which is derived from Player. I need to make sure ive gotten this inheritance down right.
Have you actually tried compiling this?
You have an error in your code and it's to do with char arrays - someone around here has a signiture with a suitable comment about that (can't remember who off the top of my head).
Although your inheritance is syntactically correct, it's not logical: an enemy is not a player (unless your definition of player is something other than the user's avatar). Taking the tradiational meaning of player, there will be certain (lots) of things that players do that enemies will not do. There will also be lots of things enemies do that players will never do (AI for one!). But there is a lot of stuff about players and enemies that are common. So, it makes logical sense to introduce a third, common base class, call it Character:
When designing classes, think along the lines of 'has a' and 'is a'. The latter is an inherited relationship ("a player is a character"), the former is a member variable relationship ("a player has an inventory").
Skizz
You have an error in your code and it's to do with char arrays - someone around here has a signiture with a suitable comment about that (can't remember who off the top of my head).
Although your inheritance is syntactically correct, it's not logical: an enemy is not a player (unless your definition of player is something other than the user's avatar). Taking the tradiational meaning of player, there will be certain (lots) of things that players do that enemies will not do. There will also be lots of things enemies do that players will never do (AI for one!). But there is a lot of stuff about players and enemies that are common. So, it makes logical sense to introduce a third, common base class, call it Character:
class Character{// stuff common to both players and enemies};class Player : public Character // a player is a character!{// put player specific stuff in here};class Enemy : public Character // an enemy is also a character, but definately not a player!{// put enemy specific stuff in here};When designing classes, think along the lines of 'has a' and 'is a'. The latter is an inherited relationship ("a player is a character"), the former is a member variable relationship ("a player has an inventory").
Skizz
Private variabeles can't be accessed in inherited classes, so your code is of little use right now. Also, deriving an enemy of a player class isn't the most... logical thing to do if you ask me. I'd say, use a Character class to derive both the Enemy and Player class from.
I would also recommend clearer variabele names. What's the difference between 'm_hits' and 'hits', and what is that 'treshold' for? Making your code easier to read and comprehend will help you on the long run, and believe me, code as young as 3 months old can look pretty puzzling if you don't pay attention to this. :)
As for the classes themselves, I'd say they're pretty useless at all. The getStat function doesn't return anything, for example! And you can set the name of the character, but you can never retreive it. You're using classes as data buckets here rather than actual objects that hold some logic of their own.
For example, right now you would have to write code elsewhere that retreives the stats of the player, picks the right int from the returned array (which isn't the most clear approach anyway), modifies it, and stores the stats back into the player. Why shouldn't you give the player class the responsibility for thise? Give it an ApplyDamage(int damage) function and let that function make sure that the health value gets modified, and never drops below 0 or gets above the health maximum, and that the isDead boolean is set to true when the health reaches 0.
Think of classes as more than just data storages. Think of them as data storages with an interface, that hides the inner complexities of that class for the rest of the code. Code outside the player class shouldn't need to know at which array index the health is stored. You shouldn't even allow any other class to just change any statistic when they wanted to: it defeats the whole use of 'private'.
Oh, and you may want to write the class constructors (and destructor) yourself. To make sure things get initialized as desired, rather than at default values.
Other than those design issues, it looks like a syntactically correct inheritance to me. :)
I would also recommend clearer variabele names. What's the difference between 'm_hits' and 'hits', and what is that 'treshold' for? Making your code easier to read and comprehend will help you on the long run, and believe me, code as young as 3 months old can look pretty puzzling if you don't pay attention to this. :)
As for the classes themselves, I'd say they're pretty useless at all. The getStat function doesn't return anything, for example! And you can set the name of the character, but you can never retreive it. You're using classes as data buckets here rather than actual objects that hold some logic of their own.
For example, right now you would have to write code elsewhere that retreives the stats of the player, picks the right int from the returned array (which isn't the most clear approach anyway), modifies it, and stores the stats back into the player. Why shouldn't you give the player class the responsibility for thise? Give it an ApplyDamage(int damage) function and let that function make sure that the health value gets modified, and never drops below 0 or gets above the health maximum, and that the isDead boolean is set to true when the health reaches 0.
Think of classes as more than just data storages. Think of them as data storages with an interface, that hides the inner complexities of that class for the rest of the code. Code outside the player class shouldn't need to know at which array index the health is stored. You shouldn't even allow any other class to just change any statistic when they wanted to: it defeats the whole use of 'private'.
Oh, and you may want to write the class constructors (and destructor) yourself. To make sure things get initialized as desired, rather than at default values.
Other than those design issues, it looks like a syntactically correct inheritance to me. :)
It looks ok so far although just an observation about this line : Edit missed the error in the Player class
enmType getRace(const enmType &);
Does this function need to take an argument?
Did you know that you can also put enums inside of class declarations and then you can always test like this
enmType getRace(const enmType &);
Does this function need to take an argument?
Did you know that you can also put enums inside of class declarations and then you can always test like this
switch (EnemyPtr->getRace()){ case Enemy::Archer: { } case Enemy::Dragon: { } // Etc...}/* This means that you can free up those names in the global namespace so you can then use those names if you plan on making each enemy type into an object You can then take out your setRace function out of your Enemy class and use the constructor initialisation list to initialise your race variable like so*/class Enemy : public Player{public: enum enmType { Archer, Dragon, Brigand, Bandit, Hellhound, Giant Tick, Daemon };protected: Enemy(enmType racevar) : Race(racevar) {}};class Archer : public Enemy{public: Archer() : Enemy(Enemy::Archer) {}};class Dragon : public Enemy{};
Author
yeah, i was just making Enemy a derivation of Player to make things a bit simpler, though i do agree that now that i look at it differently, it can be a bit confusing.
"m_hits" stands for "max_hits", and "hits" is "current hits".
Threshold is kind of like a Limit Gauge (FF7 i believe), when you take a certain amount of damage (5-10% of max or so), its incremented, and once it hits its maximum (whoops, lol, forgot that var), you get a damage bonus alongside a couple of other stats. I'm gonna comment all that stuff.
i also need to differentiate from GetStat when you want an int, and GetStat when you want to get your character's name.
oh, i had another question, is there a function i can use to get the length (minus the nulls) of a char array? im pretty sure there IS one, just not too sure on the name.
constructors and destructors will be in there too, just havent put them in yet (dont know exactly WHY theyre not in there now).
haha, i need to make the stats PROTECTED dont i?
is this better? i revised it to fix some issues.
[Edited by - RavynousHunter on October 16, 2006 9:21:02 PM]
"m_hits" stands for "max_hits", and "hits" is "current hits".
Threshold is kind of like a Limit Gauge (FF7 i believe), when you take a certain amount of damage (5-10% of max or so), its incremented, and once it hits its maximum (whoops, lol, forgot that var), you get a damage bonus alongside a couple of other stats. I'm gonna comment all that stuff.
i also need to differentiate from GetStat when you want an int, and GetStat when you want to get your character's name.
oh, i had another question, is there a function i can use to get the length (minus the nulls) of a char array? im pretty sure there IS one, just not too sure on the name.
constructors and destructors will be in there too, just havent put them in yet (dont know exactly WHY theyre not in there now).
haha, i need to make the stats PROTECTED dont i?
is this better? i revised it to fix some issues.
class Character{ protected: int str, dex, con, intel, will, gold, exp, next_level, level, threshold, atk, def, magic, res, hits, max_hits, mana, max_mana; /* str = Strength | con = Constitution intel = Intelligence | exp = Experience atk = Attack | magic = Magic Attack res = Resistance (defense against magic) def = Defense (defense against physical damage) threshold = Your character's breaking point */ char name[55]; public: void Character(); void ~Character(); int getStat(const char[10]); void setStat(char[10]); void setName(char[55] cName) { name = cName; } char getName() { return name; }};class Enemy : public Character{ enum enmType { Archer, Dragon, Brigand, Bandit, Hellhound, Giant Tick, Daemon }; private: enmType Race; public: void Enemy(); void ~Enemy(); enmType getRace(const enmType &); void setRace(enmType cRace) { Race = cRace; }};class Player : public Character{ public: void Player(); void ~Player();};[Edited by - RavynousHunter on October 16, 2006 9:21:02 PM]
To get the length of a char array you need to use the function strlen
char* ptr = "String";
len=strlen(ptr);
However, I notice that you are doing
void getStat(char[10]);
This will not work.
Instead try
void getStat(char* pStat);
You could look into the STL string class
The header to include is
#include <string>
and to use the string
std::string name;
name="Mr Ed";
To get the length
len=name.length();
char* ptr = "String";
len=strlen(ptr);
However, I notice that you are doing
void getStat(char[10]);
This will not work.
Instead try
void getStat(char* pStat);
You could look into the STL string class
The header to include is
#include <string>
and to use the string
std::string name;
name="Mr Ed";
To get the length
len=name.length();
I want to enforce Adam's post, and tell you that you should must use std::string instead of char arrays. They are easier to use, and less error prone (and your current level shows that you should use simpler things).
To use them, simply #include <string> and delcare all the strings as std::string instead of char[] or char*.
List of operations that are easier to do with std::string:
* creation: std::string mystring = "my value";
* assignation mystring = "another value";
* getting length: size_t length = mystring.length();
* is-equal comparison: if (mystring == "some value") { }
* is-different comparison: if (mystring != "some value") { }
* destruction: implicit, when it goes out of scope the string is destroyed automagically.
Old C-like function that use a const char* as the parameter can still be used, by calling the c_str() method of std::string:
You can still access the different characters using the [] operator or the at() method if you want to do bound checking:
(the latter is likely to produce an out_of_bound exception that would need to be catched).
Finding a substring is done using of of the std::string::find() method.
Getting a substring is done using the substr() method.
And so on. As you can see, they are quite easy to handle.
Now, let's speak about your code.
First, avoid as possible the declaration of long variable lists as you do. Instead of "int str, dex, con, intel, will, gold, exp" and so on, use only one line per variable, as it makes the code easier to read. You should also use a correct naming scheme, in order to simplify the reading (mana vs. m_mana). I also stronlgy encourage you to use a prefix or a suffix to your member variables, in order to differentiate them from local variables or function parameters.
What about getAttribute() and getName()? Again, use correct names for your functions. Not everything is a stat - a name is not a stat. Moreover, it will probably be better to add one function for each attribute: getStrength(), getAttack() and so on. This will prove to be easier in the future - because you are more likely to create logic bugs using a single function whose paramter is only checked at run-time than by using a function whose name is verified at compile time (and that will spot a compilation error if it is incorrectly spelled).
With std::string, you can get the length of the string using the length() method, as posted by Adam Hamilton.
If you need a comment to explain what is the goal of your variable, then something is wrong - you'll better find a better name. Don't worry about name length - most modern code editors have some auto-completion features (in VC++, this is triggered by [ctrl]+[space]) so you won't type them each time you want to use them.
The same "that's not a good name" remark goes for a lot of other elements: enmType, atk, def, hits vs. m_hits, mana vs. m_mana.
About setName(): is the name of a Player supposed to change while the game is running? If not then the name should be a parameter of the class constructor, and you shouldn't have a setName() method.
On the inheritance subject, as other posters told you, you got it wrong. Inheritance should not be used to reuse code. The Liskov Substitution Principle (yeah, I know - big name) states that if a class inherit another one, a function that takes the base class as a parameter should be able to use the inherited class without even knowing it. Your Enemy class will probably have to be handled differently from the Player class, so if a function takes a Player* as the parameter, it is likely to need modifications if you feed it with a Enemy class.
In fact, I'm not even sure that you even need an inheritance relation between the two classes (even something like what Skizz did might be debatable). The reason is that while they share some similar information, the behavior of those classes are very different - making Character class a class without defined behavior. It is probably far better to store the needed information in a stat block object and to use composition to create your classes:
Voilà - no inheritance, and no relation between an Ennemy and a Player, while keeping the stat block separately (and thus making it a good candidate for reuse). No need to tackle with private, public and so on - and best of all: a correct encapsulation.
This can be enhanced by using Adam Hamilton's approach to define new ennemy types (as the current idea is severly limited: if you add a new ennemy type, you'll need to modify your code greatly, and you can suffer about this later: imagine you have only 50 ennemy type, and prey [smile]). Adam's approach is more robust and less limited. It also satisfy the Open/Closed Principle (another big word, I know) which states that code should be open to extension but closed to modification (ie code you write should not be modified when you want to extend its behavior; of course, this is not always possible, but this should be enforced whenever possible).
If you have further questions about software design, let us know - we'll try to help you [smile]
Regards,
To use them, simply #include <string> and delcare all the strings as std::string instead of char[] or char*.
List of operations that are easier to do with std::string:
* creation: std::string mystring = "my value";
* assignation mystring = "another value";
* getting length: size_t length = mystring.length();
* is-equal comparison: if (mystring == "some value") { }
* is-different comparison: if (mystring != "some value") { }
* destruction: implicit, when it goes out of scope the string is destroyed automagically.
Old C-like function that use a const char* as the parameter can still be used, by calling the c_str() method of std::string:
some_function_that_needs_a_const_char_ptr(mystring.c_str());You can still access the different characters using the [] operator or the at() method if you want to do bound checking:
mystring[0] = 'K'; mystring.at(15) = 'e';(the latter is likely to produce an out_of_bound exception that would need to be catched).
Finding a substring is done using of of the std::string::find() method.
Getting a substring is done using the substr() method.
And so on. As you can see, they are quite easy to handle.
Now, let's speak about your code.
First, avoid as possible the declaration of long variable lists as you do. Instead of "int str, dex, con, intel, will, gold, exp" and so on, use only one line per variable, as it makes the code easier to read. You should also use a correct naming scheme, in order to simplify the reading (mana vs. m_mana). I also stronlgy encourage you to use a prefix or a suffix to your member variables, in order to differentiate them from local variables or function parameters.
// attributes:int mStrength;int mDexterity;int mConstitution;// and so on.Quote:i also need to differentiate from GetStat when you want an int, and GetStat when you want to get your character's name.
What about getAttribute() and getName()? Again, use correct names for your functions. Not everything is a stat - a name is not a stat. Moreover, it will probably be better to add one function for each attribute: getStrength(), getAttack() and so on. This will prove to be easier in the future - because you are more likely to create logic bugs using a single function whose paramter is only checked at run-time than by using a function whose name is verified at compile time (and that will spot a compilation error if it is incorrectly spelled).
Quote:oh, i had another question, is there a function i can use to get the length (minus the nulls) of a char array? im pretty sure there IS one, just not too sure on the name.
With std::string, you can get the length of the string using the length() method, as posted by Adam Hamilton.
Quote:Threshold is kind of like a Limit Gauge (FF7 i believe), when you take a certain amount of damage (5-10% of max or so), its incremented, and once it hits its maximum (whoops, lol, forgot that var), you get a damage bonus alongside a couple of other stats. I'm gonna comment all that stuff.
If you need a comment to explain what is the goal of your variable, then something is wrong - you'll better find a better name. Don't worry about name length - most modern code editors have some auto-completion features (in VC++, this is triggered by [ctrl]+[space]) so you won't type them each time you want to use them.
The same "that's not a good name" remark goes for a lot of other elements: enmType, atk, def, hits vs. m_hits, mana vs. m_mana.
About setName(): is the name of a Player supposed to change while the game is running? If not then the name should be a parameter of the class constructor, and you shouldn't have a setName() method.
On the inheritance subject, as other posters told you, you got it wrong. Inheritance should not be used to reuse code. The Liskov Substitution Principle (yeah, I know - big name) states that if a class inherit another one, a function that takes the base class as a parameter should be able to use the inherited class without even knowing it. Your Enemy class will probably have to be handled differently from the Player class, so if a function takes a Player* as the parameter, it is likely to need modifications if you feed it with a Enemy class.
In fact, I'm not even sure that you even need an inheritance relation between the two classes (even something like what Skizz did might be debatable). The reason is that while they share some similar information, the behavior of those classes are very different - making Character class a class without defined behavior. It is probably far better to store the needed information in a stat block object and to use composition to create your classes:
class StatBlock{private: int mStrength; int mDexterity; std::string mName; // ...public: // the : mName(name) is the initialization list. You can read more // about this on wikipedia or on the web. Suffice to say that it inits // mName with the value of name at constructio time. Of course, it // can only be used in a constructor. StatBlock(const std::string& name) : mName(name) { } void setStrength(int strength) { mStrength = strength; } void setDexterity(int dexterity) { mDexterity = dexterity; } int getStrength() const { return mStrength; } int getDexterity() const { return mDexterity; } std::string getName() const { return mName; } // the later should be const std::string& getName() const { return mName; } // but I don't want to botehr you with return-by-value vs. return-by-ref // or const correctness at this point};class Player{private: StatBlock mStats;public: Player(const std::string& name) : mStats(name) { } void setStrength(int strength) { mStats.setStrength(strength); } void setDexterity(int dexterity) { mStats.setDexterity(dexterity); } int getStrength() const { return mStats.getStrength(); } int getDexterity() const { return mStats.getDexterity(); } std::string getName() const { return mStats.getName(); }};class Ennemy{public: // I declare the enum as part of the Ennemy class. enum EnnemyType { Dragon, Bandit, Daemon, };private: EnnemyType mType; StatBlock mStats;public: // both the name and the ennemy type are unlikely to change while // the game is running - so let's construct the ennemy using // these values as the constructor parameters. Ennemy(Ennemy::EnnemyType type, const std::string& name) : mType(type), mStats(name) { } int getStrength() const { return mStats.getStrength(); } int getDexterity() const { return mStats.getDexterity(); } std::string getName() const { return mStats.getName(); } Ennemy::EnnemyType getType() const { return mType; }};Voilà - no inheritance, and no relation between an Ennemy and a Player, while keeping the stat block separately (and thus making it a good candidate for reuse). No need to tackle with private, public and so on - and best of all: a correct encapsulation.
This can be enhanced by using Adam Hamilton's approach to define new ennemy types (as the current idea is severly limited: if you add a new ennemy type, you'll need to modify your code greatly, and you can suffer about this later: imagine you have only 50 ennemy type, and prey [smile]). Adam's approach is more robust and less limited. It also satisfy the Open/Closed Principle (another big word, I know) which states that code should be open to extension but closed to modification (ie code you write should not be modified when you want to extend its behavior; of course, this is not always possible, but this should be enforced whenever possible).
If you have further questions about software design, let us know - we'll try to help you [smile]
Regards,
In this specific instance it makes sense to not have any relationships between Player and Enemy (stats are a 'have a' relationship), but there are plenty of instances when a common base class does make sense (and fulfil the Liskov Substitution Principle). It all depends on what you want to do with the system. Which means up-front design. For example, say you want to have a grenade type weapon and that weapon can inflict damage and physics effects on objects (players and enemies) in the game when it detonates. It makes sense then to have a 'Damagable' base class. This does introduce type identification problems: for a collection of objects, which are the Damagable ones? RTTI solves this but has overhead. You could use a message based system and all objects implement a MessageHandler base class. There are other solutions as well. These are decisions that need to be made early on in the design process.
Skizz
Skizz
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
