Properly Store Object in Vector

Question

I'm brand new to C++, and this is my first time posting (recipe for disaster). I've spent about a day trying to resolve my problem/question, in the mean time finding no easily recognizable solution on the forums. It's possible that my question has already been asked or that a solution has already been posted but I've overlooked it or misunderstood it. A similar post exists here, but it's not obvious to me what to do with the information.

I'll provide a concise, high-level summary of the point of this code. I'll then ask the specific question(s) to which I'm unable to find an answer, and I'll follow with the code I've written.

SUMMARY: I'm creating a program to help do bookkeeping for a game. The game may have any number of players, and each player having a small list of attributes/members (e.g. playerName, playerAllegiance, etc.) which are elements of a Player class/object. The user is first asked to input the name of each player (enteredName), and the program must create a new Player object for each name that's entered. This seems to be appropriately handled by a dynamic array, so I chose to use a vector (called playerIndex) to store each Player object. A for-loop allows the user to input names, each name instantiating a new Player object which is to be stored (copied?) into playerIndex using vector::push_back. At the conclusion of the for-loop, the user should be left with a vector of Player objects, each Player object storing a name in its playerName member.

QUESTION/PROBLEM: The code seems to work proprly when monitoring the vector inside the aforementioned for-loop. Immediately after the user enters the Nth player's name, the program spits out the playerName string stored in the Nth element of playerIndex using a Player class function getPlayerName() [actual code: playerIndex[playerCounter].getPlayerName()]. As soon as the user enters a blank playerName (i.e. presses enter without entering a name), it indicates the user has entered all player names, so the for-loop terminates. Just after this loop, a loop which is designed to output the playerName of each Player object stored in playerIndex does not output the expected names. I don't know why this is happening, but based on my very minimal knowledge of constructors, I'm guessing it has something to do with a copy or move constructor for the Player class. Can anyone clearly explain how to handle this issue? I fear I may be making a pitifully stupid, novice mistake and/or misunderstanding a key concept of C++.

CODE: This code is cropped/simplified to be as clear as possible. For example, the Player class is shown to have only one member (playerName) while, in the original code, it has four or five other members.

//HeaderPlayerClass.hpp
#include <iostream>
#include <string>

#ifndef PLAYERCLASS_HPP
#define PLAYERCLASS_HPP

using std::string;

class Player {
    private:
        string *playerName;

    public:
        Player();
        Player(string);
        ~Player();
        string getPlayerName();
};

#endif




//PlayerClass.cpp
#include "HeaderPlayerClass.hpp"
#include <iostream>
#include <string>

using std::string;

Player::Player() {
    playerName = new string;
}

Player::Player(string enteredName) {
    playerName = new string;    
    *playerName = enteredName;
}

Player::~Player() {
    delete playerName;
}

string Player::getPlayerName() {
    return *playerName;
}





//main.cpp
#include <cstdio>
#include <iostream>
#include <string>
#include <vector>
#include "HeaderPlayerClass.hpp"

using std::cin;
using std::cout;
using std::string;
using std::vector;

int main(int argc, char** argv) {

    string buffer;
    vector<Player> playerIndex;

    int playerCounter = 0;

    for(;;) {
        if(playerCounter==0) {
            cout << "\nEnter player name and press enter; leave blank and press enter to continue.\n";
        }

        cout << "\nPlayer " << playerCounter+1 << ":";
        getline(cin, buffer);

        if(buffer == "esc") {
            cout << "PROGRAM EXITED BY USER\n";
            return 0;
        }

        if(buffer.empty()) {
            break;
        }

        playerIndex.push_back(Player(buffer));
        cout << "Player " << playerCounter+1 << "'s name:" << playerIndex[playerCounter].getPlayerName() << "\n";

        ++playerCounter;
    }

    for(int ii = 0 ; ii < playerIndex.size() ; ii++) {
        cout << "\nThis should display player " << ii+1 << "'s name:" << playerIndex[ii].getPlayerName();
    }

    return 0;
}

Answer 1

class Player {
    private:
        string *playerName;

Do not store a pointer to the string, store the string itself

class Player {
    private:
        string playerName;

Constructor

Player::Player() {
    playerName = new string;
}

This is unnecessary if you store the string itself - the default constructor will initialize it for you.

Here is where the problems start - this will leave you vulnerable to memory leaks unless you carefully write a destructor

Player::Player(string enteredName) {
    playerName = new string;    
    *playerName = enteredName;
}

All you need, if you store the string itself:

Player::Player( const string& enteredName)
: playerName ( enteredName )
{}

You constructor is really scary. Just leave things to the default destructor when you store the string itself

Player::~Player() {
    delete playerName;
}

Next, you are working too hard keeping your own counter. std::vector maintains its own count and using

playerIndex.size()

will save trouble and bugs