Question

I have the following classes:

class Player
{
public:
    Inventory& GetInventory()
    {
        return inventory;
    }
private:
    Inventory inventory;
};

class Inventory
{
    // Methods removed for brevity
private:
    std::vector<std::string> items;
};

What I'm trying to do is allow a user of the code to modify the contents of Player::inventory without being able to reassign it to a different object. Short example:

Player player;

// OK: I want to able to add items.
player.GetInventory().AddItem("Item 1");

// Not OK: I don't want to be able to assign
// Player::inventory to a different Inventory.
Inventory badInventory;
player.GetInventory() = badInventory;

I know I can accomplish this by modifying GetInventory() to return Inventory* const, but this scenario is making me doubt what I believe to be true of references. I believe they:

  1. Are not objects themselves, so do not have an address.
  2. Cannot be reassigned during their lifetime.
  3. Must be initialised when declared.

That says to me that badInventory must somehow be getting copied implicitly. So I defined a copy constructor, hoping to log the call, like so:

Inventory(const Inventory& rhs)
{
    std::cout << "Copy ctor." << std::endl;
}

But that never fires. So my question boils down to: what is happening on this line?

player.GetInventory() = badInventory;

I'm missing something here, but I don't know what it is.

Was it helpful?

Solution

That line is actually calling the copy assignment operator, not the copy constructor. That's the function declared as

Inventory& operator=(const Inventory&);

Note that player.GetInventory() is an object that already exists, so it cannot invoke the copy constructor, simply because you're not constructing anything.

If you want to forbid inventories from being copied, delete these functions:

Inventory(const Inventory&) = delete;
Inventory& operator=(const Inventory&) = delete;

Or, if your compiler lacks C++11 support, declare them as private and don't provide a definition.

private:
    Inventory(const Inventory&);
    Inventory& operator=(const Inventory&);

OTHER TIPS

In this scenarion

player.GetInventory() = badInventory;

there is used the copy assignment operator for class Inventory.

You can define copy/move assignment operators as deleted. For example

class Inventory
{
public:
    Inventory & operator =( const Inventory & ) = delete;
    Inventory & operator =( Inventory && ) = delete;
    // Methods removed for brevity
private:
    std::vector<std::string> items;
};

Or you can declare them as private member functions.

Though in my opinion it would be better if you would define all required methods of processing the vector as member functions instead of the returning of reference to the vector

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top