CODEWITHSUNDEEP
c++vectoriterator
t354
t354

Reputation: 617

Erasing elements of a vector within two for loops

I am iterating over the rows of a 2D array (lib) and comparing the first 4 entries in each row to a vector of tuples (near_pts) containing 4 elements. Basically, I want to extract all rows from lib where the first 4 elements (in that row) match any of the tuples in near_pts, and add these rows to a new 2D array (sub_lib). There should not be any repeats in lib or near_pts.

When a tuple from near_pts is matched in lib, I want to erase it from near_pts so that no time is wasted trying to match that particular tuple. I would expect that, since I have a break statement immediately after the erase, we would go to the next iteration of the exterior for loop and the iterator over near_pts would be reset to handle the modified version of near_pts. However, this doesn't seem to be happening, a few of the tuples are never matched (and there should always be a match). I know the problem is related to the iterator because my debugging efforts have shown that the iterator sometimes only loops over 1 element near_pts when multiple elements still exist, but I cannot figure out why this happening. The code is below, please let me know if more information and/or clarity is needed.

int n = 0;
for (int i=0; i<numPts; i++) {
  for (vector<my_tup>::iterator it = near_pts.begin(); it != near_pts.end(); it++) {
    bool match = (get<0>(*it)==lib[i][0] && get<1>(*it)==lib[i][1] &&
                  get<2>(*it)==lib[i][2] && get<3>(*it)==lib[i][3]);

    // If there is a match, add it to the sub-library, erase the entry
    // from near_pts, and exit the interior loop.
    if (match) {
      for (int j=0; j<numLibCols; j++) { sub_lib[n][j] = lib[i][j]; }
      n++;
      near_pts.erase(it);
      break;
    }
    // If we have found all of the tuples, exit the loop.
    if (n==near_pts.size()) { break; }
  }
}

Notes: lib is effectively a 2D array of size numPts x 13, near_pts is a vector of my_tup, where my_tup is a tuple< double,double,double,double >, and sub_lib is a 2D array of size near_pts.size() x 13, where this size is set before any of the elements of near_pts are erased.

Upvotes: 0

Views: 113

Answers (3)

ECrownofFire
ECrownofFire

Reputation: 462

Erasing during iteration in a vector invalidates the iterator, so you need to update it. Doing this also eliminates the check for n at the end because when near_pts is empty, the iterator must be at near_pts.end().

int n = 0;
for (int i=0; i<numPts; i++) {
  vector<my_tup>::iterator it = near_pts.begin();
  while(it != near_pts.end()) {
    bool match = (get<0>(*it)==lib[i][0] && get<1>(*it)==lib[i][1] &&
                  get<2>(*it)==lib[i][2] && get<3>(*it)==lib[i][3]);

    // If there is a match, add it to the sub-library, erase the entry
    // from near_pts, and exit the interior loop.
    if (match) {
      for (int j=0; j<numLibCols; j++) { sub_lib[n][j] = lib[i][j]; }
      n++;
      it = near_pts.erase(it);
      break;
    }
    else {
      ++it;
    }
  }
}

Upvotes: 1

Dietmar K&#252;hl
Dietmar K&#252;hl

Reputation: 153820

Using

near_pts.erase(it);

invalidates it. Any use of the iterator it after this operation has undefined behavior. You may want to use

near_ptrs.erase(it++);

instead: this way the iterator it is moved off the erased element before it is erased. Of course, you can't unconditionally increment it after using that statement.

Upvotes: 0

Michael Simbirsky
Michael Simbirsky

Reputation: 3103

Your final condition

// If we have found all of the tuples, exit the loop.
if (n==near_pts.size()) { break; }

is incorrect as near_pts gets decreased and n gets increased on each match.

You probably want to check something like if (near_pts.empty()) break;

Upvotes: 2

Related Questions