Am I writing my code efficiently?

I started writing a base to a text base adventure game and I was wondering since I am pretty far in that if I am writing the code correctly and efficiently. I would like some criticism. I began writing the program to improve on different aspects of C++, mostly classes, inheritance, pointers, etc.

https://github.com/MoBaT/TextAdventureBase/tree/master/TextAdventureBase
Last edited on
Are you writing your code efficiently or are you writing efficient code?
First thing I saw is why do Gear Class has the same statistics as Character class? What is the significance of Gear::dexterity, for example?

Mike has the following members:
1
2
3
4
5
6
7
8
9
Mike
    - Character:
        - Gear:
            - Gear::strength
            - Gear::dexterity 
            - Gear::etc...
        - Character::strength
        - Character::dexterity
        - Character::etc


Next thing:
1
2
3
4
5
6
7
8
9
10
11
12
13
class Inventory
{
public:
	enum type {WEAPON, ARMOR, CONSUMABLE, MISCELLANEOUS, SKILL};
	Inventory(void);
	~Inventory(void);
	bool addItem(Skill* item);
	bool addItem(Miscellaneous* item);
	bool addItem(Consumable* item);
	bool addItem(Weapon* item);
	bool addItem(Armor* item);
	void addItem(int);
	// (...) 


This is exactly where you should use inheritance. There really should be only one function bool addItem (ParentItemClass * item) instead of 6 functions (that is 6 untill you think of adding another item subtype). (I may, of course, misinterpret your intentions).
Last edited on
I would assume the character strength, dex, etc is the level of each of those values the character itself has, and for the gear, the level of each of those values that the gear either grants to the character or is required for the gear?
Yes GTM, when you equip weapons, it goes into the vector which is in the gear class. Each weapon or armor has individual stats and when you add armor and weapons to your gear, then it adds to the gear overall strength, dex etc. Then I can update the Character class to temporarily add the stat items to the characters stats.

Also Jock, I don't understand what you mean by having one function bool addItem (ParentItemClass * item). The Weapon, Armor and Consumable all inherit from the item class but the only thing all these inherits in the std::string itemName. If I were to do something like Item* item and set it equal to my new Armor(values) then it would only add the name of the item to my Inventory, however, i also need the attributes of that item. There may be better ways to implement this, but that was the only thing I can think of at the time. Please give me suggestions to improve my classes, it would be grateful.

I could create a new class called CharacterStats and then use it to where my character has the object and make the gear have the object aswell, would that work?
Last edited on
closed account (D4S8vCM9)
If you have a few bucks left buy "Design Patterns" by the Gang of Four. Apart that it is the quasi standard book on design patterns, the choice of examples is fantastic and for your game the maze examples could be a good pointer.

http://www.amazon.com/Patterns-Elements-Reusable-Object-Oriented-Software/dp/0201633612

(This is no ad, but a real recommendation!)

If you are only interested in efficiency and performance, run it through cppcheck. It will show up alle the forgotten references to objects (i.e. foo(std::string) instead of foo(const std::string &)) and much more interesting static code checks.

I did it for you with that command cppcheck . --enable=all --std=posix -I.:
The interesting output is: [TextAdventureBase/Character.h:20]: (error) Memory leak: Character::gear. A quick look over Character.cpp revealed you didn't delete it in the destructor.

For an in-depth analysis, sorry, I have not the mood after over 10 hours of developing today.
Last edited on
Topic archived. No new replies allowed.