[C++] 1 level of inheritance problems

Started by
47 comments, last by sheep19 15 years, 9 months ago
Hi, I'm making a text RPG. I decided to begin from the easy ones and make the weapon class. I want it to be abstract. Then I have a sword class, which is inherited from weapon. Finally, I have a dagger class which is inherited from sword. Weapon -> Sword -> Dagger I want both the weapon and the sword classes to be abstract. I'm doing something like this.. Weapon.h

#ifndef WEAPON_H
#define WEAPON_H

class Weapon
{
public:
	Weapon( int health = 100, int power = 1, int value = 0, int weight = 1 );
	~Weapon();
	virtual void viewAttributes() const = 0;

protected:
	int* m_pHealth, *m_pPower, *m_pValue, *m_pWeight;
};

#endif

Weapon.cpp

#include "Weapon.h"

Weapon::Weapon(int health, int power, int value, int weight)
{
	m_pHealth = new int( health );
	m_pPower = new int( power );
	m_pValue = new int( value );
	m_pWeight = new int( weight );
}

Weapon::~Weapon()
{
	delete m_pHealth;	m_pHealth = 0;
	delete m_pPower;	m_pPower = 0;
	delete m_pValue;	m_pValue = 0;
	delete m_pWeight;	m_pWeight = 0;
}

Sword.h

#ifndef SWORD_H
#define SWORD_H

class Weapon;

class Sword : public Weapon
{
public:
	Sword( int health = 100, int power = 1, int value = 0, int weight = 1 );
	~Sword();
	virtual void viewAtrributes() const = 0;
};

#endif

Sword.cpp

#include "Weapon.h"
#include "Sword.h"

Sword::Sword(int health, int power, int value, int weight ): Weapon( health, power, value, weight )
{
}

Dagger.h

#ifndef DAGGER_H
#define DAGGER_H

class Weapon;
class Sword;

class Dagger : public Sword
{
public:
	Dagger( int health = 100, int power = 1, int value = 0, int weight = 1 );
	virtual void viewAttributes() const;
};

#endif

Dagger.cpp

#include <iostream>

#include "Weapon.h"
#include "Sword.h"
#include "Dagger.h"

Dagger::Dagger(int health, int power, int value, int weight): Sword( health, power, value, weight )
{
}

void Dagger::viewAttributes() const
{
	std::cout << "Your dagger's attributes:\n"
		<< "Health: " << *m_pHealth << " %\n"
		<< "Power: " << (*m_pHealth) * (*m_pPower) << "\n"
		<< "Value: " << *m_pValue << " gold\n"
		<< "Weight: " << *m_pWeight << " kilograms\n"
		;
}

main.cpp

#include <iostream>

#include "Weapon.h"
#include "Sword.h"
#include "Dagger.h"

int main()
{
	Weapon* w = new Dagger( 100, 10, 84, 12 );
}

--------- First, I want to know if this is good design (I mean the whole idea of how objects relate). If not, what should I do? Another question is: I had to include 4 files in main. Is there to way to reduce that number; because it will increase when I add more types of weapons/swords etc. So, I'm getting some errors: 1>------ Build started: Project: PROJECT1, Configuration: Debug Win32 ------ 1>Compiling... 1>main.cpp 1>f:\my documents\visual studio 2005\projects\project1\project1\main.cpp(9) : error C2259: 'Dagger' : cannot instantiate abstract class 1> due to following members: 1> 'void Sword::viewAtrributes(void) const' : is abstract 1> f:\my documents\visual studio 2005\projects\project1\project1\sword.h(11) : see declaration of 'Sword::viewAtrributes' 1>Build log was saved at "file://f:\My Documents\Visual Studio 2005\Projects\PROJECT1\PROJECT1\Debug\BuildLog.htm" 1>PROJECT1 - 1 error(s), 0 warning(s) ========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ========== Thanks in advance and sorry for the long post.
Advertisement
Maybe a bit unrelated but why is a dagger a kind of a sword? Couldn't a sword be a really long dagger?
The problem is that Sword doesn't implement viewAttributes()
You have a typo: viewAtrributes

Other than that, I agree with the others. As-is, a dagger has no special qualities over a sword, other than the name. Adding a name and or description member to weapon, and I see no reason for any derived classes.

Also, don't use dynamic allocation of the members, that is needlessly complex:
class Weapon{public:	Weapon( int health, int power, int value, int weight, const std::string &name );	        // Make destructors virtual if the class will be a super class        // however, you may not need one        // ~Weapon();	void viewAttributes() const;private:	int health, power, value, weight;        std::string name;};// weapon.cppWeapon::Weapon(int health, int power, int value, int weight, const std::string &name):    health(health),    power(power),    value(value),    weight(weight),    name(name){}void Weapon::viewAttributes() const{	std::cout << "Your " << name << "'s attributes:\n"		<< "Health: " << health << " %\n"		<< "Power: " << (health * power) << "\n"		<< "Value: " << value << " gold\n"		<< "Weight: " << weight << " kilograms\n";}
Quote: Also, don't use dynamic allocation of the members, that is needlessly complex:


At least for primitive data types :) Dynamic allocation of members can occasionally be useful, i.e if you want to make your object out of 'components' which you want to behave polymorphically.

Edit :-
I couldnt previously think of a good example to reinforce this. However now I have :)...say you wanted to make your swords upgradable, then if inside your sword object you have a pointer to a Handle object and you also have a pointer to a Blade object then you could swap and change your blades and handles to make whatever kind of sword you liked at runtime :)

[Edited by - chipmeisterc on May 15, 2008 12:09:12 PM]
You may want to re-name your Sword class to be a Blade class. It makes sense since a Blade is-a Weapon, and a Dagger is-a Blade. The Blade class could cover things like the blade losing its edge, re-sharpening the blade, the blade breaking, etc. Your breakdown of your weapons seem like a good idea to me thus far.

I also agree with rip-off regarding the dynamic allocation.
Beginners are often in the habit of wildly overusing inheritance. This is usually because they don't fully understand the power of composition-based object specialization. Inheritance is difficult to get right the first time, and getting inheritance wrong is the number one thing you can do to screw up early on in a manner that only becomes evident much later. In your case, I would strongly advise against having this multi-level inheritance strategy. It looks fun and elegant now, but please believe me that later it will bite you in the ass and never let go.

BTW, why aren't you including Sword.h in Dagger.h?
Wow, thanks for the replies. I really can't check them out now, it's 11.10 pm here :)
Alright. I have corrected the typo and it runs fine now.

As Mantear suggested, I will rename the Sword class to Blade.

Now, about the Dagger class...Well, I did it in order to have different types of Swords (Blades now). I am thinking of, like rip-off said, to delete the Dagger class and have a string object in the Blade class to distinguish the types. Maybe having a vector of strings with all the names in..picking a random one in some circumstances and check in a function the blade type to set its attributes. Something like this maybe:
void setAttributes( Sword* const b ){	if( b->type == "Dagger" )	{		b->m_pHealth = 1;		/*....*/	}}

Regarding the above code, it might be a good idea not to have default arguments in the constructor..but having pre-set attributes for each weapon/blade type.

-------------------------------------------------------------------------------

Actually, that was what I intented to do. Like saying:
Weapon* myDagger = new SamuraiDagger(); or something like this.

But this will need even more inheritance... My question now is: Why having multiple levels of inheritance will be a problem?

Edit: why having my data members on the heap isn't good? Isn't it supposed to be more efficient?
Quote:Original post by sheep19
Actually, that was what I intented to do. Like saying:
Weapon* myDagger = new SamuraiDagger(); or something like this.

But this will need even more inheritance... My question now is: Why having multiple levels of inheritance will be a problem?


It's added complexity. You have to be sure that the benefits it gives you outweigh the complexity.

Personally, I think that since most weapons have exactly the same behaviour, and only differ in quantitative terms, they should be all one class.

Weapon* myDagger = new Weapon(TYPE_DAGGER, "SamuraiDagger"); perhaps.

Quote:Edit: why having my data members on the heap isn't good? Isn't it supposed to be more efficient?


Quite the opposite. You've doubled the memory requirement (since you now need a pointer as well as the original value) and have to spend time allocating and deallocating memory.

This topic is closed to new replies.

Advertisement