C++ deleting a list member while iterating: standard solution is not working?

Question

Here's my problem. I've read many previous questions about how to delete a member of a list while iterating over it and I tried the various solutions that the answers proposed. It happens that they seem not to work. I have a list of classes of this kind:

class Walker {
    public:
        Walker(int); 
        ~Walker(); 

        double *x; 
        double *y; 
        double *z; 
        double weight; 
        int molteplicity; 
};

The constructor and destructor are the following

Walker::Walker(int particle_num) {
    x = new double[particle_num];
    y = new double[particle_num];
    z = new double[particle_num];
}

Walker::~Walker() {
    delete x;
    delete y;
    delete z;
}

Now, the list

list<Walker> population;

is defined as a member of another class. Now, if the element molteplicity is null (calculated via another function) I have to dinamically remove the member from the class, and this is how I do it:

for( it = population.begin(); it != population.end(); ) {
        if( it->molteplicity == 0 ) {
            it = population.erase(it);
        } else {
            ++it;
    }

getting the following error at runtime:

prog(22332) malloc: * error for object 0x7f838ac03a60: pointer being freed was not allocated * set a breakpoint in malloc_error_break to debug Abort trap: 6

Do you see the error? Thank you very much for your help!! If you need some more code just let me know.

Answer 1

The problem has nothing to do with the use of std::list, but it's in the destructor:

Walker::~Walker() {
    delete x;
    delete y;
    delete z;
}

You have allocated using new[], not new, therefore you have to use delete[] not delete:

Walker::~Walker() {
    delete[] x;
    delete[] y;
    delete[] z;
}

Live demo

Notice also that molteplicity and weight are never initialized and can therefore contain any number (likely different from 0).

After those changes you program compiles and runs perfectly.

---

Also notice that new and delete are generally frowned upon in the C++ community for very good reasons. Use smart pointers or containers, and please, generally follow the rule of zero.

And finally you can reach a cleaner solution using std::list::remove_if. If you follow these tips, you'll get something along the lines of:

struct Walker {
    Walker(int num) 
        : x(num), y(num), z(num)
        , weight(0)
        , molteplicity(0)
        {}

    std::vector<double> x, y, z; 
    double weight; 
    int molteplicity; 
};

and used as:

std::list<Walker> population {...};
population.remove_if([](Walker const& w) { return w.molteplicity == 0; });

Live demo

Which is both more readable and more correct.