C++ OOP - Losing data

Question

I have a few simple classes and I can't get them to work.

TL;DR I have a "Player" instance, after I set some data to the instance, I can get it back. If I push the instance to std::vector Players; if I have Players.at(0).getName() it returns "". The data is not there! Is gone. (Debugging the application I see "_name" set in "vPlayer" and in "Players" I see an element, with "_name" = "")

Here is the code:

//Player.h
#ifndef PLAYER_H
#define PLAYER_H

#include <iostream>

class Player
{
public:
    Player();
    Player(const Player &Player);
    Player& operator=(const Player &Player);
    std::string getName();
    bool        setName(const std::string &name);
    bool        nameValid(const std::string &name);

private:
    std::string _name;
};



#endif



//Player.cpp

#include "Player.h"
#include <iostream>
#include <string>
using namespace std;

Player::Player()
{

}
Player::Player(const Player &Player)
{

}
Player& Player::operator=(const Player &Player) {
    return *this;
}

std::string Player::getName()
{
    return this->_name;
}

bool Player::setName(const std::string &name)
{
    if ( ! this->nameValid(name) )
    {
        return false;
    }

    this->_name = name;
    return true;
}

bool Player::nameValid(const std::string &name)
{
    return name.empty() == false;
}




//Map.h
#ifndef MAP_H
#define MAP_H

#define MAP_X 40
#define MAP_Y 40

#include "Player.h"
#include "Point.h"
#include <vector>

class Map
{
public:
    Map();
    bool movePlayer(Player &Player, Point &Point);
    std::vector<Player> getPlayers();
private:

};

#endif //MAP_H



//Map.cpp

#include "Map.h"
#include "Player.h"
#include "Point.h"
#include <iostream>
#include <string>

using namespace std;

Map::Map()
{

}

bool Map::movePlayer(Player &Player, Point &Point)
{
    return true;
}
std::vector<Player> Map::getPlayers()
{
    Player vPlayer;
    vPlayer.setName(std::string("test"));
    std::vector<Player> Players;

    Players.push_back(vPlayer);

    return Players;
}

in main:

  std::vector<Player> Players = vMap.getPlayers();
  cout<<"Test:"<<Players.at(0).getName()<<endl;

Check out the top answer to http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom for some info on copying, moving and assigment. — tecu, Jan 22 '13 at 15:21
When you're calling `Players.push_back(vPlayer);` you're not adding your current instance of vPlayer to the vector but a copy of it instead. Since your copy constructor is empty, the `_name` field in the copy is also empty. — Alberto Miranda, Jan 22 '13 at 15:22
You could lose a couple of `this->` in this code, as they are not necessary (and actually quite uncommon in C++). Yes, explicit is usually better than implied, but in this case it makes the code "strange". — DevSolar, Jan 22 '13 at 15:31

score 10 · Answer 1 · answered Jan 22 '13 at 15:20

You define the class's copy constructor and copy assignment operator to do nothing. How do you expect the copy in the vector to have same data as the instance you put into the vector?

Your class can be perfectly fine with the default, compiler-generated copy constructor and copy assignment operator, so just remove your declarations and definitions for them and everything will work.

score 4 · Accepted Answer · answered Jan 22 '13 at 15:23

Your vector will contain copies of the elements you add to it. These copies will be added using Player::Player(const Player&) constructor.

This constructor (in your implementation) doesn't set any value for the name.

Solutions:

set the name in the copied object:

Player::Player(const Player &Player) : _name(Player._name) { }

(The same is true for your assignment operator)

Remove the copy and assignment functionality and rely on the default. Because the name is a std::string, it will get a copy of the source player name by default.

C++ OOP - Losing data

2 Answers2