CODEWITHSUNDEEP
c++vectoriterator
limlim
limlim

Reputation: 357

Erasing an element from the vector during iteration c++

I wrote this method to find the minor of a sparse matrix:

SpMatrixVec SparseMatrix::minor(SpMatrixVec matrix, int col) const{

    SpMatrixVec::iterator it = matrix.begin();

    int currRow = it->getRow();

    int currCol = col;

    while(it != matrix.end()) {

        if(it->getRow() == currRow || it->getCol() == currCol){
            matrix.erase(it);
        }
        // if we have deleted an element in the array, it doesn't advance the
        // iterator and size() will be decreased by one.
        else{   
            it++;
        }

    }

    // now, we alter the cells of the minor matrix to be of proper coordinates.
    // this is necessary for sign computation (+/-) in the determinant recursive
    // formula of detHelper(), since the minor matrix non-zero elements are now
    // in different coordinates. The row is always decreased by one, since we
    // work witht he first line, and the col is decreased by one if the element
    // was located after 'col' (which this function receives as a parameter).

    //change the cells of the minor to their proper coordinates.
    for(it = matrix.begin(); it != matrix.end(); it++){

        it->setRow(it->getRow()-1);

        int newY;
        newY = (it->getCol() > col) ? it->getCol() + 1 : it->getCol();

        it->setCol(newY);
    }
    return matrix;

}

Now, i'm probably doing something wrong, because when reaching the second interation of the while loop, the program crashes. The basic idea was to go over the vector, and see if it is the relevant coordinate, and if so - to delete it. I increment the iterator only if there was no deletion (and in this case, the vector should update the iterator to be pointing the next element..unless i got these things wrong).

Where is the problem?

Thanks a lot.

Upvotes: 2

Views: 3421

Answers (3)

ereOn
ereOn

Reputation: 55796

erase() invalidates your iterator.

You must update it using the return value of erase() for the loop to work:

while(it != matrix.end()) {

    if(it->getRow() == currRow || it->getCol() == currCol){
        //matrix.erase(it);
        it = matrix.erase(it); // Here is the change
    }
    // if we have deleted an element in the array, it doesn't advance the
    // iterator and size() will be decreased by one.
    else{   
        //it++;
        ++it; // ++i is usually faster than i++. It's a good habit to use it.
    }

}

Upvotes: 9

Benoit
Benoit

Reputation: 79233

erase invalidates your iterator. Do it = matrix.erase(it) instead.

Upvotes: 6

vulkanino
vulkanino

Reputation: 9134

You can't change a collection while you're iterating between its elements. Use another temp collection to store the partial results.

edit: Even better, use a functor to delete elements: remove_if You write the condition.

Upvotes: -2

Related Questions