Remove an object from an array of pointers

Question

I have an array of pointers that point to Building objects that look like this:

class Building {
    int id ;
    int year;
    double price;
    char* adress;
};

What I'm trying to do is to remove objects from the array via the id using operator-, but I don't really know how to do so. My list class looks like this:

class List {
    Building* buildingList[10];
    static int buildingNr;
    
public:
    
    List operator-(int id) {
        bool exists = false;
        int index;
    
        for(int i = 0; i < buildingNr; i++)
            if (buildingList[i]->getId() == id) {
                exists = true;
                index = i;
                cout << "Building " << index << " was deleted." << endl;
            }
    
        if (exists) {
            delete buildingList[index];
            buildingList[index] = buildingList[buildingNr];
            buildingList[buildingNr] = NULL;
            buildingNr--;
        }
        else throw - 1;
    } 
};
    
int List::buildingNr = 0;

I know that there are far easier ways of doing this, like using std::vector, but this is an assignment and I have to do it using the overloaded operator- and an array of pointers, and the Building class has to look like that.

I also have the operator+ which adds an element to the array, and that works fine.

Not 100% clear ion what you're asking, but assuming `buildingNr` is the number of items in the list, `buildingList[buildingNr]` is going to be out of range. You could `buildingNr--` first or `buildingList[buildingNr-1]`. — user4581301, Dec 09 '20 at 23:50
Some handy reading: [What are the basic rules and idioms for operator overloading?](https://stackoverflow.com/questions/4421706/what-are-the-basic-rules-and-idioms-for-operator-overloading) The wisdom contained therein will help you get the basic shape of your operator right. — user4581301, Dec 09 '20 at 23:50
Replace the `char *` with `std::string`. Character arrays are a pain, especially when you have to dynamically allocate them for a structure. — Thomas Matthews, Dec 09 '20 at 23:53
One question to ask yourself before overloading an operator is "Does this behaviour make sense?" I doubt you have much say in this matter since this is homework, but always something to consider. Does making an operator `-` that performs a find-and-remove make the source code easier to read? — user4581301, Dec 09 '20 at 23:55
First I made a method for removing items, but the teacher explicitly said we have to overload the - operator to remove objects from the list. The problem is that I don't know how to remove an object from an array of pointers, since it is dinamicaly allocated. Should I use delete/delete[] or none of them and just set the pointer to NULL? — ANDU, Dec 10 '20 at 00:00

Remy Lebeau · Accepted Answer · 2020-12-10T00:18:40.653

Your operator- is not implemented correctly. Although its loop to find an object is fine, removing that object is broken. You are not updating the array correctly, as you need to shift down ALL of the array elements after the found index, which you are not doing.

Also, make sure your List class implements the Rule of 3/5/0 correctly. The return value of operator- is supposed to return a new List object, so a copy has to be made. You should not be modifying the this object at all in operator- (that is the job of operator-= to do; see What are the basic rules and idioms for operator overloading?).

Also, the buildingNr member should not be static. That prevents you from using multiple List objects at a time.

Try something more like this instead:

#include <algorithm>

class List {
    Building* buildingList[10];
    int buildingNr;
    
public:
    
    List() : buildingNr(0) {}

    List(const List &src) : buildingNr(src.buildingNr) {
        for(int i = 0; i < buildingNr; ++i) {
            buildingList[i] = new Building;
            buildingList[i]->id = src.buildingList[i]->id;
            buildingList[i]->year = src.buildingList[i]->year;
            buildingList[i]->price = src.buildingList[i]->price;
            buildingList[i]->adress = new char[strlen(src.buildingList[i]->adress)+1);
            strcpy(buildingList[i]->adress, src.buildingList[i]->adress);
        }
    }

    ~List() {
        for(int i = 0; i < buildingNr; ++i) {
            delete[] buildingList[i]->adress;
            delete buildingList[i];
        }
    }

    List& operator=(const List &rhs) {
        if (this != &rhs) {
            List tmp(rhs);
            std::swap_ranges(buildingList, buildingList+10, tmp.buildingList);
            std::swap(buildingNr, tmp.buildingNr);
        }
        return *this;
    }

    List& operator-=(int id) {    
        for(int i = 0; i < buildingNr; ++i) {
            if (buildingList[i]->getId() == id) {
                delete buildingList[i];
                for(int j = i + 1; j < buildingNr; ++j) {
                    buildingList[j - 1] = buildingList[j];
                }
                buildingList[buildingNr] = NULL;
                --buildingNr;
                cout << "Building " << i << " was deleted." << endl;
                return *this;
            }
        }
        throw -1;
    } 

    ...
};

friend List operator-(List lhs, int id) {    
    lhs -= id;
    return lhs;
}

Thanks a lot, the throw -1 was there so I i could treat an exception in case the id given was inexistent. — ANDU, Dec 10 '20 at 00:08

Remove an object from an array of pointers

1 Answers1