Is there a better way of writing this c++ code

Question

*I have very little skill in c++. I'm a 17 year old programmer attempting to learn c++.

I am attempting to make a fun project to help me learn c++. This piece of code is some of that result. I am creating a derived class from an equipment base class called Weapon_definition. Within it I want to take the variables from my base class into this class and give them new values when a certain weapon is called. So if shortsword is called then the weapon class object changes its values. This weapon class object is then called in another function for the explicit purpose of copying its values somewhere else. A inventory system pretty much is what this all is. The question itself though is this, is there a better or simpler way to write this? I wrote it this way with the idea that i could forever extend it and it wouldn't take much code to add or remove pieces. New sections like equipable jewelry or armor i could make with a new extern class object and class.

Header File

#ifndef EQUIPMENT_H
#define EQUIPMENT_H
#include <string>

class Equipment_definition{
  public:
  int value;
  int growth;
  std::string Tag;
  std::string Title;
};

class Weapon_definition: public Equipment_definition{
  public:
  static void Short_Sword();
  static void Long_Sword();  
};

extern Weapon_definition Weapon;



#endif

CPP File

#include "Equipment.h"

Weapon_definition Weapon;

void Weapon_definition::Short_Sword(){
  Weapon.Title = "Shortsword";
  Weapon.Tag = "[Light Weapon]";
  Weapon.value = 2;
  Weapon.growth = 1;
}

void Weapon_definition::Long_Sword(){
  Weapon.Title = "Longsword";
  Weapon.Tag = "[Heavy Weapon]";
  Weapon.value = 5;
  Weapon.growth = 0;
}

"_Is there a better way of writing this c++ code_" - Sure. The first thing would be to make it compile. Make a [mcve]. — Ted Lyngmo, Nov 18 '19 at 20:51
Normally, your class hierarchy would be along the lines of a base-class `Weapon` with descendants such as `Long_sword`. You would not have "equipment definitions." — Mike Robinson, Nov 18 '19 at 20:58
Consider using an [enumerated type](https://en.cppreference.com/w/cpp/language/enum) instead of a string for `Tag`. Get some [solid reference materials](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). The code example given tells me that whatever you're learning from right now isn't doing you any favours. — user4581301, Nov 18 '19 at 20:58
Is this your first time programming using OO? It seems that you should take a look in basic concepts like [Inheritance](https://www.tutorialspoint.com/cplusplus/cpp_inheritance.htm) — braindf, Nov 18 '19 at 20:59
I'd skip a weapon inheritance hierarchy in favour of the structure the asker's using. There isn't much of a difference between a long sword and a short sword that a few descriptive strings and a different damage range wouldn't cover. Making too many classes quickly leads to a management nightmare. Save the inheritance for changes in behaviour, not in the metadata. — user4581301, Nov 18 '19 at 21:01
@user4581301 A longsword can't be wielded by all character classes and there may be more weapon class specifics that needs to be put somewhere :-) — Ted Lyngmo, Nov 18 '19 at 21:09
@GabrielConnell I don't have enough to write a full answer, but this could give some ideas: https://godbolt.org/z/7WvW47 — Ted Lyngmo, Nov 18 '19 at 21:21
@TedLyngmo I'd break the items down into categories and the character classes would know what item categories they can use. The item would just know what category it is and the character class would have a virtual `can_use` method that tests the category. `enum` is more than sufficient to describe the categories unless extreme configurability is required. — user4581301, Nov 18 '19 at 21:22
@user4581301 That may be so. One can start out like you suggest and break it up further only if the need arises. — Ted Lyngmo, Nov 18 '19 at 21:24
@TedLyngmo There will be inheritance at some point. Polyjuice Potion is just too different from Long Sword. Flaming Long Sword might have different behaviours (double damage to ice Monsters, etc...). The question is how do you handle stuff like +5 Holy Avenger? Does it have its own item category so only the Paladin can wield it or do you make the category system smarter and more expensive? You don't want to have to rewrite a tonne of code every time you add an item to the game... — user4581301, Nov 18 '19 at 21:33
...but you also don't want to push out a configuration error where the Chef Class can't use the +9 Cheese Grater because someone spelled the name wrong. — user4581301, Nov 18 '19 at 21:33
@user4581301 :-D Some big design decisions has to be made for sure! — Ted Lyngmo, Nov 18 '19 at 21:35
My goodness, I have a lot to learn haha. Thank you all for the amazing feedback. — Gabriel Connell, Nov 19 '19 at 14:11
@MikeRobinson I wished to make a equipment_definition as I didn't want to make too many separate classes to handle if I have armor or jewelry. I thought maybe I could have jewelry, armor, and weapons all as different subclasses of the same base class, equipment. Though this is probably a bad design. — Gabriel Connell, Nov 19 '19 at 14:19

Yamahari · Answer 1 · 2019-11-18T21:29:11.523

Using factory and builder pattern combined you could do something like this

#include <string>

class Effect;
class Modifier;

class Equipment {
 public:
  class Builder final {
   public:
    Builder& setValue(int value);
    Builder& setGrowth(int value);
    Builder& setTitle(const std::string& title);
    Builder& setTag(const std::string& tag);
    Builder& addEffect(const Effect& effect);
    Builder& addModifier(const Modifier& modifier);
    Builder& copy(const Equipment& equipment);
    Equipment build();
  };
};

class EquipmentFactory {
 public:
  virtual void generateEquipment(Equipment::Builder& builder) = 0;
};

class ShortSwordFactory final : public EquipmentFactory {
 public:
  void generateEquipment(Equipment::Builder& builder) override {
    builder.setValue(2)
        .setGrowth(1)
        .setTitle("Shortsword")
        .setTag("[Light Weapon]");

    // .addEffect(...)
    // .addModifier();
  }
};

class LongSwordFactory final : public EquipmentFactory {
 public:
  void generateEquipment(Equipment::Builder& builder) override {
    builder.setValue(5)
        .setGrowth(0)
        .setTitle("Longsword")
        .setTag("[Heavy Weapon]");

    // .addEffect(...)
    // .addModifier();
  }
};

I'll take this into consideration! I first need to read about this factory and builder pattern but this seems quite nice, thank you! — Gabriel Connell, Nov 19 '19 at 13:52

Is there a better way of writing this c++ code

1 Answers1