for (Shape *i : shapes) { for (Shape *j : shapes) { if (i != j) { if (check(i,j)){ shapes.erase(remove(shapes.begin(), shapes.end(), i), shapes.end()); this causes an error because it's going to carry on iterating even though i does not exist, my question is how do I cleanly do this? currently I get an error "vector iterator not incrementable"
Can i just exit the second loop and continue in the first one?
You cannot erase elements from a vector when you are iterating it by for range loop, as internally it uses iterators that would be invalidated. This should work:
auto end = shapes.end(); for( auto it = shapes.begin(); it != end; ++it ) { end = shapes.erase( std::remove_if( std::next( it ), shapes.end(), [it]( Shape *s ) { return check( *it, s ); }, shapes.end() ); } Note this code is slightly more effective than yours but it assumes that check( s1, s2 ) == check( s2, s1 ), if you can change your check() function to do strict ordering comparison, rather than equivalence, then you would be able to use std::sort and std::unique which are even more effective.
You can't modify the positioning of your shapes elements while using ranged-based for loops. The range loop uses iterators internally, and erasing vector elements invalidates existing iterators.
Try something more like this instead:
auto iter = shapes.begin(); auto end = shapes.end(); while (iter != end) { auto iter2 = shapes.begin(); bool erased = false; while (iter2 != end) { if ((iter != iter2) && check(*iter, *iter2)) { iter = shapes.erase(iter); end = shapes.end(); erased = true; break; } ++iter2; } if (!erased) ++iter; } Alternatively, maybe something more like this would also work:
shapes.erase( std::remove_if(shapes.begin(), shapes.end(), [shapes&](Shape *i) { for (Shape *j : shapes) { if ((i != j) && check(i, j)) { return true; } } return false; } ), shapes.end() ); 
vector, so you have to be very careful with how you manage the iterators. Doing a second erase will affect the new iterators obtain from the first erase. Do you really want to remove multiple elements per loop iteration?check() is actually checking, and whether j can ever be before i in the check result, or will j always be after i.You cannot use a range-for loop in this case. Instead use a standard loop with iterators:
for (auto iter = shapes.begin(); iter != shapes.end(); iter++)
