Vector erase iterator outside of range

Question

link to the project I am creating a small glut application that will draw circuit components. I have a class called "Component" that looks like that:

// nextIndex is the index of the next component to be created
int Component::nextIndex = 0;
Component::Component( void ){
setType( RESISTOR );
setValue( 0.0 );
setX( 0.0 );
setY( 0.0 );
setSize( 2.0 );
setIndex( nextIndex );
}

eType Component::getType( void ){return cType}
double Component::getValue( void ){return cValue;}
double Component::getX( void ){return cX;}
double Component::getY( void ){return cY;}
double Component::getSize( void ){return cSize;}
void Component::setType( eType type ){cType = type;}
void Component::setValue( double value ){cValue = value;}
void Component::setX( double x){cX = x;}
void Component::setY( double y ){cY = y;}
void Component::setSize( double size ){cSize = size;}
int Component::getIndex( void ){return index;}
void Component::setIndex( int n ){index = n;}

NOTE: I am incrementing the nextIndex every time I create a new instance of the class. Here is my other class which has a vector of Components and handles the addition and deletion of the components:

class OGWindow{
private:

    bool LINK, DELETE;
    int linkNo;
    ...
    Component currentSquare;
    Component *currentSquarePtr, *linkSquPtr1, *linkSquPtr2;
    double squareXStart, squareYStart, squareXEnd, squareYEnd;
    std::vector< Component > compVector;

public:
    ...
    void myMouseClick(int button, int state, int x, int y);
    ...
    void deleteSquare( Component *squPtr );
};

I am creating components to the window fine like that: int NEXT_INT = 0;

if (x >= createButtonX && x <= createButtonX + createButtonWidth) {
    cout << "Clicked Create button" << endl;
    Component *compPtr = new Component;
    compPtr->setX(0.0);
    compPtr->setY(0.0);
    compPtr->setIndex(NEXT_INT);
    NEXT_INT++;

    compVector.push_back(*compPtr);
    //------------SET THE CURRENT SQUARE TO THE NEWLY CREATED COMPONENT-----
    vector<Component>::iterator constIterator;
    for (constIterator = compVector.begin(); constIterator != compVector.end(); ++constIterator){
        currentSquarePtr = &(*constIterator);
    }
    cout << "Vector size:  " << compVector.size() << endl;
    glutPostRedisplay();
}

But when I am deleting, it deletes some of the elements and at the last one I get the "vector out of range" error:

void OGWindow::deleteSquare( Component *squPtr ) {
    compVector.erase(compVector.begin() + (squPtr->getIndex()));
cout << "Vector size:  " << compVector.size() << endl;
glutPostRedisplay();
}

Please share any ideas why this is happening.

Answer 1

There's massive memory leaking going on when you create components. You allocate a new Component on the heap, and then put a copy of that into the std::vector. After scope exit, the pointer that was pointing to the new component on the heap is lost forever.

Just create components on the stack, add those to your vector and you'll be fine.

Furthermore, indices start at 0, not at 1:

int Component::nextIndex = 1;

should be

int Component::nextIndex = 0;

But this is a design mistake anyway; when you remove a component from your std::vector, you will have to update all the indices of the components that are lying after the deleted component (you need to decrease their value by 1).

It would be better if you start thinking in terms of iterators instead of pointers. I'm guessing you want the following: just a gigantic global datastructure where all the components reside. This could be an std::list<Component> compList. Then forget about managing indices and pointers, and use std::list<Component>::iterator everywhere instead of Component*. Removing a component is then safe, simply by doing

void OGWindow::deleteSquare( std::list<Component>::iterator squPtr ) {
    compList.erase(squPtr);
    cout << "List size:  " << compList.size() << endl;
    glutPostRedisplay();
}

and to add a component

if (x >= createButtonX && x <= createButtonX + createButtonWidth) {
    cout << "Clicked Create button" << endl;
    Component comp;
    comp.setX(0.0);
    comp.setY(0.0);

    // Notice how we put the new component at the front
    // instead of the back.
    compList.push_front(comp);

    //------------SET THE CURRENT SQUARE TO THE NEWLY CREATED COMPONENT-----
    // easy now!
    currentSquarePtr = compList.begin();

    cout << "List size:  " << compList.size() << endl;
    glutPostRedisplay();
}

Remember, change these

Component *currentSquarePtr;
Component *linkSquPtr1;
Component *linkSquPtr2;

to these

std::list<Component>::iterator currentSquarePtr;
std::list<Component>::iterator linkSquPtr1;
std::list<Component>::iterator linkSquPtr2;