Character class for an RPG game

Question

I have an RPG-like program that uses a Character class to specify and return information about an individual player/character. I was curious about a way to best set and return an inventory of strings in terms of memory and performance for this purpose. I gave some thought to creating a struct or using something similar to the following:

char *inventory[] = { "Item", "Item I", "Item II", "Item III" };
    for (int i = 0; i < 4; ++i) {
        char *pos = inventory[i];
            while (*pos != '\0') {
                printf("%s ", *(pos++));
            }

And setting *inventory[] in the above from a single array containing all strings in the program. My goal would be to return all the inventory from a player in the most efficient way possible, while still retaining the ability to choose individual items (through the same or another function); though this does seem problematic for scaling above 4 items, or when the size is changed.

How else can this be improved?

#include <iostream>
#include <string>
using std::cerr;
using std::cin;
using std::cout;
using std::endl;
using std::string;

class Character {

private:

string name, classType;
int experience;
string inventory[4];

public:
// Should be separate?
struct Position {
    int x; int y;
};
Position location;

public:

void Character::setName(string x) { name = x; }
string Character::getName() { return name; }

void Character::setClassType(string x) { classType = x; }
string Character::getClassType() { return classType; }

void Character::setExperience(int x) { experience = x; }
int Character::getExperience() { return experience; }

void Character::setPosition(int x, int y) {
    location.x = x; location.y = y;
}
Position& Character::getPosition() { return location; }

void Character::setInventory(string(&x)[4]) { for (int i = 0; i < 4; ++i) { inventory[i] = x[i]; } }
string& Character::getInventory(int slot) { return inventory[slot]; }

};

void showUser(Character player);

int main() {

    bool running(true);

    while (running) {

        try {

            string itemsWizard[4] = { "Scroll of Invisibility", "Mana Potion", "Enchanted Staff", "Mage Robe" };
            string itemsRanger[4] = { "Longbow", "Quiver", "Dagger", "Jerkin" };
            string itemsWarrior[4] = { "Broadsword", "Shield", "Steel Breastplate", "Throwing Axe" };

            Character characterI, characterII, characterIII;

            characterI.setName("Jax");
            characterI.setClassType("Ranger");
            characterI.setExperience(4750);
            characterI.setInventory(itemsRanger);
            characterI.setPosition(50, 20);

            characterII.setName("Brand");
            characterII.setClassType("Warrior");
            characterII.setExperience(9999);
            characterII.setInventory(itemsWarrior);
            characterII.setPosition(1, 45);

            characterIII.setName("Paige");
            characterIII.setClassType("Sorcerer");
            characterIII.setExperience(5900);
            characterIII.setInventory(itemsWizard);
            characterIII.setPosition(65, 120);

            cout << "\n" << "Retrieving Character Info..." << "\n" << endl;
            showUser(characterI);
            showUser(characterII);
            showUser(characterIII);

        } // Close try

        // Generalized Catch
        catch (std::exception & e) {
            cerr << "\nERROR : " << e.what() << '\n';
        }

        char choice('c');
        bool invalidChoice(true);
        // Option to run again
        while (cout << "\nRun Again? (y/n) \n\n" && (!(cin >> choice)) || (!(choice == 'n')) && (!(choice == 'y'))) {
            cin.clear();
            cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
            cerr << "\nERROR: The only valid answers are y/n. \n" << endl;
        }
        if (choice == 'y') {
            invalidChoice = false;
            cout << "\nSelected: \"" << choice << "\" *** RUNNING AGAIN. ***\n" << endl;
        }
        else if (choice == 'n') {
            invalidChoice = false;
            running = false;
            cout << "\nSelected: \"" << choice << "\" *** EXITING. ***\n" << endl;
        }

    }

    // Pause before returning
    do {
        cin.ignore();
        cout << '\n' << "Press enter to continue...";
    } while (cin.get() != '\n');

    return 0;

}// End Main

void showUser(Character player) {
    cout << "----------------------------------------" << endl;
    cout << "Name : " << player.getName() << endl;
    cout << "Class : " << player.getClassType() << endl;
    cout << "Experience : " << player.getExperience() << endl;
    Character::Position p = player.getPosition();
    cout << "Location : (" << p.x << "," << p.y << ")" << endl;
    for (int i = 0; i < 4; ++i) {
        cout << "Inventory " << i + 1 << " : " << player.getInventory(i) << endl;
    }
}

Answer 1

Use a vector instead of a raw array

Your C-like code would be unlikely to be reviewed by any modern C++ expert. The issue is too many beginners focus on trivialities and premature optimization rather than actually understanding the language. Here's how to turn your suggestion into a one-liner:

// a and b are both std::vector<std::string>'s
a = b;

You'll quickly realize that the implementators of your standard library spent a lot more time thinking about it than you have and whatever ad-hoc approach you think is "faster" will beat by it.

Getters and setters

Getters and setters used in this fashion are an anti-pattern. It creates a bulky interface, invites mistakes¹ and gives the impression of somebody who looked at Java or C# code without understanding the implications. Your class can be cut down to:

struct Character
{
    std::string name, classType;
    int experience;
    std::vector<std::string> inventory;
    Position location;
};

Now it can benefit from aggregate initialization:

Character character{"Test", "Test", 42, {"a", "b", "c"}, {1, 2}};

See how much simpler that is?

This also simplifies assignment:

// a and b are both Character's
a = b;

Following the "Rule of Zero", memberwise assignment is handled automagically.

^{Footnote 1}

void Character::setInventory(string(&x)[4]) { for (int i = 0; i < 4; ++i) { inventory[i] = x[i]; } }
string& Character::getInventory(int slot) { return inventory[slot]; }

What's the purpose of having a get that also acts like a set? Perhaps you intended to overload operator[] instead? Not only does it do the wrong thing (it returns a slot, rather than entire inventory) but it's inconsistent with the rest of your interface. Even if you performed some operations other than "get"ing, it's implicit that "set" mutates and that "get" inspects (it doesn't modify any member variables).

Therein lies the danger of blindly applying patterns you don't understand: a bulky and incorrect interface.

Note that your "get" method doesn't do any bounds checking. std::vector's at() does do bounds checking, and throws an std::out_of_range exception.

Pointless try..catch

Is there anything that you put in the try..catch block that can throw an exception? If it did, how would you handle it? What kind of exceptions do you expect? Again, please think more carefully about what you're trying to achieve rather than blindly inserting code.