What is the best design of getting derived class property?

Question

class Item
{
// public:
//    virtual int GetDamage() const { return -1; }
};

class Weapon : public Item
{
public:
    Weapon(int InDamage)
        :Damage(InDamage)
    {
    }

    int GetDamage() const { return Damage; }
    // virtual int GetDamage() const override { return Damage; }

private:
    int Damage = 0;
};

class Armor : public Item
{
public:
    Armor(int InDefence)
        :Defence(InDefence)
    {
    }

    int GetDefence() const { return Defence; }

private:
    int Defence = 0;
};

class ItemManager
{
public:
    ItemManager()
    {
        ItemMap.insert
        ({
            {"Weapon", new Weapon(50)},
            {"Armor", new Armor(100)},
        });
    }

    Item* GetItem(const std::string& ItemName) const
    {
        const auto It = ItemMap.find(ItemName);
        return It->second;
    }

private:
    std::map<std::string, Item*> ItemMap;
};


int main()
{
    ItemManager Manager;

    Item* ItemInst = Manager.GetItem("Weapon");
//  const int Damage = ItemInst->GetDamage()

//  Weapon* WeaponInst = dynamic_cast<Weapon*>(ItemInst);
//  std::cout << WeaponInst->GetDamage() << std::endl;

    return 0;
}

this is code sample.

solutions All I have, to get weapon damage from Item are

use dynamic_cast to cast item to weapon.
add GetDamage virtual function in Item class, and override this even though armor doesn't need.

I'm really uncomfortable with these solutions.

it's like I'm trying to destroy 'SOLID design Principles'.

I'm sure, there is better solution just I don't know.

please, teach me the best code design.

thanks in advance.

`dynamic_cast` is almost always a code-smell. Using virtual methods is a viable solution. But I'm not sure it makes sense in this case to add virtual methods for all methods any derived class might need (and since `Armor` doesn't have damage, and `Weapon` doesn't have defence). The question is what do you actually need to do with a general `Item`. — wohlstad, Sep 01 '23 at 06:45
why do you need to get the weapon damage from an item? good design is driven by answering the why? and by considering the use cases, not by pondering about some classes and trying to get them "good" in isolation. Who calls `getDamage` on an `Item`? Why? — 463035818_is_not_an_ai, Sep 01 '23 at 07:08
You have to decide if all your Items should have the same interface. If this is not true for all, you should break that dependency. If all your items can have a e.g. damage, it is fine to use Item as in interface, typically implemented via virtual functions. `dynamic_cast` is bad design in this case and has in general a smell! BTW: dynamic_cast can require RTTI enabled, which may be a problem for some reasons. — Klaus, Sep 01 '23 at 07:16
[ask] advises that you introduce your question **before** posting any code. I agree with that advice in this case. Good functional specs are more reliable than inferences drawn from (possibly buggy) code. — JaMiT, Sep 01 '23 at 22:00

pptaszni · Answer 1 · 2023-09-01T08:07:23.337

use dynamic_cast to cast item to weapon.

No

add GetDamage virtual function in Item class, and override this even though armour doesn't need.

Also no. This solution would be good, if all your items can share a common interface like item->getName(); . However, in your case damage is clearly only relevant for weapons, and defence for armours. Your design would soon explode with irrelevant methods that you are forced to override.

One solution for you would be to use the common interface to get your items properties:

class Item {
public:
    virtual ~Item() = default;
    virtual int getProperty(std::string propName) = 0;
};

class Weapon : public Item {
public:
    Weapon(int dam) { _props["damage"] = dam; }
    int getProperty(std::string propName) override {
        if (!_props.count(propName)) {
            throw std::runtime_error("Property not found");
        }
        return _props[propName];
    }

private:
    std::unordered_map<std::string, int> _props;
};

You can also have an interface to get all of them, check if prop exists (without exception), etc.

Then later you might discover that even int getProperty() is not enough, if some of your items need to store their parameters as double, or std::string. You might solve it with std::variant, or std::any, or using different interfaces for different types, or just breaking the dependency - all depends on the context.

What is the best design of getting derived class property?

1 Answers1