inheritance with one base class and three subclasses

[Tony Johansson]

Hello!

Here I have one base klass and three subklasses.

I Just want to have your opinion about the design of these klasses.
I don't think that my design is good.
I think that I instead should NOT have done the base class abstract it is
so now but that is wrong I think. The base class should be concrete

Define then method fetchWeaponName, fetchAllowedAnimal and getPrice in the
base class which mean that these can be removed from the subclasses. Change
the access specifier defined as protected to private in the base class.
These three methode below will consequently be moved to the base class.

virtual string fetchWeaponName() const
{ return Weapon::weaponName; }

string fetchAllowedAnimal() const
{ return Weapon::allowedAnimal; }

virtual int getPrice() const
{ return Weapon:rice; }


//Tony



class Weapon
{
public:
Weapon(int pris, string vapen_namn, string djur) :
price(pris),weaponName(vapen_namn), allowedAnimal(djur) {}
Weapon() {}
virtual ~Weapon() {}
virtual string fetchAllowedAnimal() const = 0;
virtual string fetchWeaponName() const = 0;
virtual int getPrice() const = 0;
protected:
int price;
string weaponName;
string allowedAnimal;
};

class MooseRifle : public Weapon
{
public:
MooseRifle(int price,string weapon_name,string animal) :
Weapon(price,
weapon_name, animal) {}

MooseRifle() {}
virtual ~MooseRifle() {}

virtual int getPrice() const
{ return Weapon:rice; }

virtual string fetchWeaponName() const
{ return Weapon::weaponName; }

string fetchAllowedAnimal() const
{ return Weapon::allowedAnimal; }
private:
};

class Winchester : public Weapon
{
public:
Winchester(int price,string weapon_name,string animal) :
Weapon(price,
weapon_name, animal) {}

virtual ~Winchester() {}

virtual string fetchWeaponName() const
{ return Weapon::weaponName; }

virtual int getPrice() const
{ return Weapon:rice; }

string fetchAllowedAnimal() const
{ return Weapon::allowedAnimal; }
private:
};

class Shotgun : public Weapon
{
public:
Shotgun(int price,string weapon_name,string animal) : Weapon(price,
weapon_name, animal) {}

virtual ~Shotgun() {}

string fetchWeaponName() const
{ return Weapon::weaponName; }

virtual int getPrice() const
{ return Weapon:rice; }

string fetchAllowedAnimal() const
{ return Weapon::allowedAnimal; }
private:
};

 

[Chris Theis]

Hello!

Here I have one base klass and three subklasses.

I Just want to have your opinion about the design of these klasses.
I don't think that my design is good.
I think that I instead should NOT have done the base class abstract it is
so now but that is wrong I think. The base class should be concrete

Define then method fetchWeaponName, fetchAllowedAnimal and getPrice in the
base class which mean that these can be removed from the subclasses.
Change
the access specifier defined as protected to private in the base class.
These three methode below will consequently be moved to the base class.

[SNIP]

Apart from some technical things like passing strings as const references,
I'd say, that you should move the functions getPrice(), fetchWeaponName() &
fetchAllowedAnimal to the base class, as you already planned to do. The
reason for this is that the functionality and also the implementation is the
same for all of your classes no matter if it is a MooseRifle or a
Winchester. It's just the value which would be different. However, there is
another thing I'd suggest to re-think. At the moment you have a function
which is called fetchAllowedAnimal and this somehow indicates a temptation
to put knowledge about possible victims of the weapons into the weapon
classes. IMHO this is something which should be taken care of at a high
level because for the weapon the possible target is not much of a
difference. Whether this type can/should be used should be decided somewhere
else based on the type information or the name of the object. Hence, I'd
suggest to at least rename the function to something like fetchType. Or you
can drop it alltogether and base the usage on the name of the object because
one object could be used for more than one different targets.

HTH
Chris