Erasing element from vector of pointers messes up pointers

[vdweller]

Hello!

Before anything else, I am aware of smart pointers. I am simply trying to understand the underlying logic of pointers and strengthen my knowledge of certain C++ mechanics. Now, to the question.

I have a simple class called ITEM:

class ITEM {
    public:
        int x{0};
        int y{0};
        ITEM (int _x,int _y) : x{_x}, y{_y} {}
};

I create items using this simple code:

ITEM* item_create(int _x, int _y) {
    ITEM* i = new ITEM(_x,_y);
    list_item.push_back(i);
    std::cout<<"created item: "<<i<<"\n";
    return i;
}

There is also a JOB class which has a public member of a reference to an item pointer:

class JOB_CARRYITEM : public JOB {
    public:
        int phase{0};
        COLONIST* colonist{nullptr};
        ITEM*& item;
        int item_targetx{0};
        int item_targety{0};
        int colonist_x_after{0};
        int colonist_y_after{0};
        JOB_CARRYITEM (COLONIST* _col, ITEM*& _itm, int _itm_tx, int _itm_ty, int _col_xafter, int _col_yafter) :
            colonist{_col}, item{_itm}, item_targetx{_itm_tx}, item_targety{_itm_ty}, colonist_x_after{_col_xafter}, colonist_y_after{_col_yafter} {}
        virtual bool validatejob() {
            if ( job_done || !colonist_is_alive(colonist) || (phase==0 && !item_exists(item)) ) {
                colonist->job=nullptr;
                return false;
            }
            return true;
        }
(...)

So essentially I create an item and add the pointer to the list_item vector. Then, when a job is assigned, the ITEM*& item member of that job is set to the item we are interested in, and during the time that the job is active, the validatejob() checks, among other things, if the reference references a nullptr (it stops if it does).

At some point in the job the colonist reaches the item and it gets destroyed. The function is:

void item_destroy(ITEM*& _item) {
    int sz{list_item.size()};
    for (int i{0};i<sz;++i) {
        std::cout<<"item before: "<<i+1<<" / "<<sz<<" "<<list_item[i]<<"("<<list_item[i]->x<<")"<<"\n";
    }
    std::cout<<"deleting item: "<<_item<<"("<<_item->x<<")"<<"\n";
    sz=list_item.size();
    for (int i{0};i<sz;++i) {
        if (list_item[i]==_item) {
            list_item.erase(list_item.begin()+i);
            break;
        }
    }
    std::cout<<"deleting item...: "<<_item<<"("<<_item->x<<")"<<"\n";
    delete _item;
    _item=nullptr;
    sz=list_item.size();
    for (int i{0};i<sz;++i) {
        std::cout<<"item after: "<<i+1<<" / "<<sz<<" "<<list_item[i]<<"("<<list_item[i]->x<<")"<<"\n";
    }
}

And here is where problems arise. The debug output is:
item before: 1 / 2 0x13773f0(250)
item before: 2 / 2 0x1377480(150)
deleting item: 0x13773f0(250)
deleting item...: 0x1377480(150)

(The number in parentheses is the x coord of the item just to make sure the pointer doesn't point to garbage.)

So the problem is...initially the _item argument points to the correct item to be deleted with address 0x13773f0.
However, after I delete the vector entry containing this pointer, _item now points to the other item with address 0x1377480 !

Why is this happening? I have searched the forum for identical problems but couldn't find any. I would appreciate any insight! Again, I am trying to understand how pointers work before moving to smart pointers.

[thmm]

The erase functions of the vector returns a new iterator to the next elem after the deleted.
http://www.cplusplus.com/reference/vector/vector/erase/

[vdweller]

This doesn't tell me much since the for loop exits immediately after removing the matching item once. In addition the _item argument isn't assigned anything new within the loop.

On aside note, I refactored some code with job assignment and the problem seems to have disappeared. The program runs in Debug for ages without problems. However I am still perplexed as to how an argument gets changed from a vector element deletion...

[keskiverto]

You are right, the erase should not affect the _item.

A pointer to element would invalidate, but _item is a copy of a value that is in element:

Iterator validity
Iterators, pointers and references pointing to position (or first) and beyond are invalidated, with all iterators, pointers and references to elements before position (or first) are guaranteed to keep referring to the same elements they were referring to before the call.

Note on:

    sz=list_item.size();
    for (int i{0};i<sz;++i) {
        if (list_item[i]==_item) {
            list_item.erase(list_item.begin()+i);
            break;
        }
    }

That could be more compact and self-documenting:

auto e = std::find( list_item.begin(), list_item.end(), _item );
if ( e != list_item.end() ) list_item.erase( e );

http://www.cplusplus.com/reference/algorithm/find/