How do you delete both a pointer in a vector of pointers of Objects and the Object itself?

Question

I'm trying to code an text-based adventure game builder. I have three classes: Room, Object, and my main class. In my Room class, I have a (private) vector of pointers of Objects: vector<Object*> objectsInRoom

This keeps track of all the Objects stored in each room. I have a function called objects() in the Room class that returns objectsInRooms for when I call that vector in my main class.

vector<Object*> Room::objects() {   return objectsInRoom;  }

In my main class, in my function pickUpObject(), I've created a vector of pointers of Objects called roomObject. I call objects() in the Room class and store the Objects in objectsInRoom (which is only accessed in the Room class) in roomObject (which is accessible in my function in main). I also have a vector of Objects called allObjects that stores all the items that I want to pick up from the room and carry around with me. It has a global scope.

I want to make it so that if I pick up an item in a particular room, I add the item to allObjects, delete the pointer to that element in roomObjects (and thus the pointer to that element in objectsInRooms in the Room class), and the item itself.

My pickUpObject function is: (Room* current just tells me what room I'm in and thus what Objects I have)

void pickUpObject(vector<Object>&allObjects, Room* current)
{
    vector<Object*>roomObjects;  int length; string name; char flag;
    roomObjects = current->objects();
    length = roomObjects.size(); 

    bool repeat = true; 
    while (repeat)
    {
        if (length == 0)
        {
            cout << "There are no objects to pick up in " << current->name() << endl;
            repeat = false; 
        }
        else
        {
            cout << "There is a ";
            for (int k = 0; k < roomObjects.size() - 1; k++)
            {
                cout << roomObjects[k]->getName();
                if (length > 2)
                    cout << ", ";
            }
            if (length > 1)
                cout << " and " << roomObjects[length - 1]->getName() << " here." << endl;
            else
                cout << roomObjects[length-1]->getName() << "." << endl;

            cout << "What object do you want to pick up?" << endl;
            cin >> name;

            //this is where the deletion happens 
            for (int i = 0; i < length; i++)
                if (name.compare(roomObjects[i]->getName()) == 0)
                {                                               
                    allObjects.push_back(*roomObjects[i]);                      
                    roomObjects.erase(roomObjects.begin() + i);
                    deleteVectorContent(roomObjects, i, i + 1); 

                }
            cout << "roomObject size = " << roomObjects.size() << endl; 
            cout << "--------------------" << endl; 
            cout << "allObject size = " <<  allObjects.size() << endl; 
            for (int i = 0; i < allObjects.size(); i++)
                cout << allObjects[i].getName() << endl; 


            for (int i = 0; i < roomObjects.size(); i++)
            {
                cout << roomObjects[i]->getName() << endl; 
            }

            cout << "Do you want to pick up another object? (Y/N): "; 
            cin >> flag; 
            if (flag == 'N')
                repeat = false; 
        }
    }


}

I've looked up various posts on StackOverflow to try and resolve my dilemma. In main, I've created a method called deleteVectorContent to try and delete the pointer.

void deleteVectorContent(vector<Object*> objectVector, int start, int stop)
{
    for (int k = start; k < stop; k++)
        delete objectVector[k];
    objectVector.clear(); 
}

I've also tried 'roomObjects.remove()' to remove the item itself from that room. Whenever I compile, however, my compiler also throws me an exception. Help would be greatly appreciated.

P.S. The link to this assignment is here. If you scroll down to the "Extra Credit for the Programming Assignment" and go to the first one marked "10 points," that is what I am working on. Thank you so much for the help!

Answer 1

Room::objects() is returning a copy of objectsInRoom, so any modifications that pickUpObject() makes to that returned vector will not be applied back to objectsInRoom. You would need to make Room::objects() return a reference to objectsInRoom instead, eg:

vector<Object*>& Room::objects()
{
    return objectsInRoom;
}

void pickUpObject(vector<Object> &allObjects, Room* current)
{
    vector<Object*> &roomObjects = current->objects();
    ...
}

Otherwise, don't provide direct access to objectsInRoom at all. Introduce new methods to Room to access/remove a given Object* from its objectsInRoom, eg:

int Room::numObjects()
{
    return objectsInRoom.size();
}

Object* Room::getObject(int index)
{
    return objectsInRoom[index];
}

Object* Room::takeObject(int index)
{
    Object *obj = objectsInRoom[index];
    objectsInRoom.erase(objectsInRoom.begin()+index);
    return obj;
}

void pickUpObject(vector<Object> &allObjects, Room* current)
{
    int length = current->numObjects();
    ...
    for (int i = 0; i < length; ++i)
    {
        if (name == current->getObject(i)->getName())
        {                                               
            Object *obj = current->takeObject(i);
            allObjects.push_back(*obj);
            delete obj;
            break;
        }
    }
    ...
}

Note that allObjects is receiving copies of the actual Objects, not Object* pointers. The code you showed is leaking memory when you make a copy of *roomObjects[i] and then erase() the Object* at i without delete'ing the Object it is pointing at. If Object is so easily copyable, you can save yourself a lot of headaches by simply getting rid of all the Object* pointers and just use Object everywhere:

class Room
{
    vector<Object> objectsInRoom;
    ...
};

int Room::numObjects()
{
    return objectsInRoom.size();
}

Object& Room::getObject(int index)
{
    return objectsInRoom[index];
}

Object Room::takeObject(int index)
{
    Object obj = objectsInRoom[index];
    objectsInRoom.erase(objectsInRoom.begin()+index);
    return obj;
}

void pickUpObject(vector<Object> &allObjects, Room* current)
{
    int length = current->numObjects();
    ...
    for (int i = 0; i < length; ++i)
    {
        if (name == current->getObject(i)->getName())
        {                                               
            allObjects.push_back(current->takeObject(i));
            break;
        }
    }
    ....
}

Otherwise, don't mix Object with Object* like you are, use Object* everywhere.

If you have a fixed set of Objects for the game, I would create a global vector<Object> to hold them all, and then just pass around Object* pointers everywhere as needed. Then you don't have to worry about cleaning up memory manually at all:

vector<Object> allGameObjects;
// fill as needed...

void Room::addObject(Object *obj)
{
    objectsInRoom.push_back(obj);
}

Object* Room::takeObject(int index)
{
    Object *obj = objectsInRoom[index];
    objectsInRoom.erase(objectsInRoom.begin()+index);
    return obj;
}

void pickUpObject(vector<Object*> &allObjects, Room* current)
{
    ...
    allObjects.push_back(current->takeObject(i));
    ...
}

If you absolutely need a vector that owns Object* pointers that have to be cleaned up before the vector is destroyed, consider using vector<unique_ptr<Object>> for that, let the compiler and STL handle the hard work for you. If you ever find yourself having to write something like deleteVectorContent(), rethink your design.