C++ iterators problem

Question

I'm working with iterators on C++ and I'm having some trouble here. It says "Debug Assertion Failed" on expression (this->_Has_container()) on line interIterator++. Distance list is a vector< vector< DistanceNode > >. What I'm I doing wrong?

vector< vector<DistanceNode> >::iterator externIterator = distanceList.begin();

   while (externIterator != distanceList.end()) {

    vector<DistanceNode>::iterator interIterator = externIterator->begin();

        while (interIterator != externIterator->end()){

          if (interIterator->getReference() == tmp){

     //remove element pointed by interIterator
     externIterator->erase(interIterator);             

          } // if
    interIterator++;
  } // while
  externIterator++;
   } // while      

Answer 1

vector's erase() returns a new iterator to the next element. All iterators to the erased element and to elements after it become invalidated. Your loop ignores this, however, and continues to use interIterator.

Your code should look something like this:

if (condition)
    interIterator = externIterator->erase(interIterator);
else
    ++interIterator;  // (generally better practice to use pre-increment)

Answer 2

I'll take the liberty to rewrite the code:

class ByReference: public std::unary_function<bool, DistanceNode>
{
public:
  explicit ByReference(const Reference& r): mReference(r) {}
  bool operator()(const DistanceNode& node) const
  {
    return node.getReference() == r;
  }
private:
  Reference mReference;
};

typedef std::vector< std::vector< DistanceNode > >::iterator iterator_t;

for (iterator_t it = dl.begin(), end = dl.end(); it != end; ++it)
{
  it->erase(
     std::remove_if(it->begin(), it->end(), ByReference(tmp)),
     it->end()
  );
}

Why ?

• The first loop (externIterator) iterates over a full range of elements without ever modifying the range itself, it's what a for is for, this way you won't forget to increment (admittedly a for_each would be better, but the syntax can be awkward)
• The second loop is tricky: simply speaking you're actually cutting the branch you're sitting on when you call erase, which requires jumping around (using the value returned). In this case the operation you want to accomplish (purging the list according to a certain criteria) is exactly what the remove-erase idiom is tailored for.

Note that the code could be tidied up if we had true lambda support at our disposal. In C++0x we would write:

std::for_each(distanceList.begin(), distanceList.end(),
  [const& tmp](std::vector<DistanceNode>& vec)
  {
    vec.erase(
      std::remove_if(vec.begin(), vec.end(),
        [const& tmp](const DistanceNode& dn) { return dn.getReference() == tmp; }
      ),
      vec.end()
    );
  }
);

As you can see, we don't see any iterator incrementing / dereferencing taking place any longer, it's all wrapped in dedicated algorithms which ensure that everything is handled appropriately.

I'll grant you the syntax looks strange, but I guess it's because we are not used to it yet.