Iterate over container in either direction AND erase elements

Question

I have two std::maps:

std::map<int,int> map1;
std::map<int,int> map2;

I need to iterate over one backwards and the other forwards (because that is the pattern of data access). Whilst iterating I would like to be able to erase elements continue iterating.

I would like to use the same method.

I have seen examples using templates showing how to iterate bidirectionally, but doesn't demonstrate erasing elements (and this is important because erase() only works with forward iterators):

Iterating over a container bidirectionally

and I have seen reverse_iterator examples which erase, but they aren't bidirectional:

How to call erase with a reverse iterator using a for loop

but I would like to iterate bidirectionally and erase?

How do you intend to erase elements from a container, given only a pair of iterators? — Jonathan Wakely, Aug 13 '18 at 15:28
@JonathanWakely By passing in the container- updated question to reflect. — user997112, Aug 13 '18 at 15:29
why would you want to erase elements in a `print` method? Maybe not the best example ;) Also: What elements do you want to erase? — 463035818_is_not_an_ai, Aug 13 '18 at 15:33
@user463035818 This is an example, I'm not posting proprietary code on stackoverflow. I am looping over a container, in either direction and I wish to erase elements and continue looping. — user997112, Aug 13 '18 at 15:37
..once you pass the map, the question is moot, because now it doesnt matter anymore whether you iterate forward or backwards, you simply first erase and then loop to print — 463035818_is_not_an_ai, Aug 13 '18 at 15:37
@super It works BUT you need to use slightly different code and I don't see how this can combine with the template technique. — user997112, Aug 13 '18 at 15:38
does it have to be a template? Why not two overloads, one for forward iterators and one for backward iterators? — 463035818_is_not_an_ai, Aug 13 '18 at 15:39
@user463035818 No it doesn't have to be template, but when I asked how to iterate bidirectionally templates were suggested. I have a lot of logic in the body of my loop and I was really hoping I could do this in one function. — user997112, Aug 13 '18 at 15:41
it isnt really clear what is the problem. You can first erase elements from the map and then loop the remaining elements. To erase the elements it doesnt matter if the next loop is forwards or backwards — 463035818_is_not_an_ai, Aug 13 '18 at 15:42
@user463035818 I have two containers. The data in one should be accessed ascending, the other should be accessed descending. I cannot iterate forwards through a container where the data is stored descending as it's incredibly inefficient. Hence I'm asking how to do this bidirectionally. — user997112, Aug 13 '18 at 15:44
also its not clear why you would need different code to erase elements, of course you can pass also a backward iterator to `std::map::erase` — 463035818_is_not_an_ai, Aug 13 '18 at 15:45
@user997112 speed of iterating forward does not depend on sort criteria you are mistaken — Slava, Aug 13 '18 at 15:46
@user463035818 See here: https://stackoverflow.com/questions/37005449/how-to-call-erase-with-a-reverse-iterator-using-a-for-loop — user997112, Aug 13 '18 at 15:47
@Slava, if I want to access the elements with the highest key value, I'm not going to start iterating from the lowest key value, am I? I'd be iterating over useless data. — user997112, Aug 13 '18 at 15:48
your "see here" is a question that has answers, sorry I still dont see the problem. I really think you better show an example of the code you'd like to write and include the errors/misbehaviour you get in the question. Currently your example does not really reflect what your "problem" is (and I am not refering to the name of the `print` method ;) — 463035818_is_not_an_ai, Aug 13 '18 at 15:53
If you are iterating through the whole list, then using the right direction iterator will make your life a whole lot easier. If you are hopping between nodes in some wandering fashion, then only you will know whether the previous entry or the next entry is the one that you want. Also the question that occurs to me is how do you know if it is safe to get the previous, if you deleted the first element in the map using a forward iterator? — Gem Taylor, Aug 13 '18 at 16:22
@GemTaylor No I am not necessarily iterating through the whole list, that is why it is essential that I begin at the correct end. There is no other problem to solve here, I need to iterate from one container forwards, another container backwards and both containers may erase iterators and I would really like to do this using the same method because there's a lot of logic and I wish to avoid copy-and-paste for no reason. C++ does not seem to accommodate this and the C++ committee should fix problems like this before adding yet more stuff in. — user997112, Aug 14 '18 at 18:33
But you say "it is essential that you start at the correct end". This sounds like the difference between begin() and rbegin() to me??? — Gem Taylor, Aug 15 '18 at 09:38

Slava · Answer 1 · 2018-08-13T16:31:50.710

1

You can write a function for a std::map, std::set or std::list:

template<typename Cont, typename Pred>
void bidir_remove_if( Cont &c, Pred p, bool forward )
{
    auto b = c.begin();
    auto e = c.end();
    while( b != e ) {
       auto it = forward ? b++ : --e;
       if( p(*it) ) {
           auto end = b == e;
           ( forward ? it : e ) = c.erase( it );
           if( end ) break;
       }
    }
}

live example

note - you cannot use this function for std::vector due to invalidation of iterators (and you should not as you better use erase-remove idiom for it).

edited Aug 13 '18 at 16:31

answered Aug 13 '18 at 15:55

Slava

43,454
1
47
90

Very nice, I will try this. It's totally safe for a std::map? – user997112 Aug 13 '18 at 16:06
It should be unless you do something fancy inside predicate. But it is no different than `std::set` (except predicate will have to accept `std::pair` of course) – Slava Aug 13 '18 at 16:08
Does `e ) = c.erase( it );` really give the correct behaviour for the backward case? It seems to me you should just *discard* the returned "safe" iterator from erase(). – Gem Taylor Aug 13 '18 at 16:17
@GemTaylor in backward case `it` equals to `e` so `e` needs to be reassigned after erase so it is equal to `e = c.erase( e );` in that case – Slava Aug 13 '18 at 16:19
Yes, but `Return value1-2) Iterator ***following*** the last removed element.` – Gem Taylor Aug 13 '18 at 16:20
and the `(b != e)`? if e deletes b, then e will never equal b – Gem Taylor Aug 13 '18 at 16:24
@GemTaylor you are right, fixed code and modified example to add this condition, thanks – Slava Aug 13 '18 at 16:32
@Slava I really appreciate your answer but given the problem you initially faced I decided to go with separate iterating functions and extract as much of the common code as possible. I wish C++ could handle this problem robustly. – user997112 Aug 14 '18 at 18:35
@user997112 C++ does handle this problem robustly, when you write code you also go through debugging/fixing bugs process. I doubt you will find a language were you just write code and it is always correct. – Slava Aug 14 '18 at 18:38
@Slava If you think this is robust then we can agree to disagree. – user997112 Aug 14 '18 at 22:37
@user997112 are you not happy that I only wrote code for you (full compile code with live example) for free but not covered with enough testcases and did not write formal proof this code is correct? – Slava Aug 14 '18 at 22:39
@Slava i'm not referring to your code, i'm referring to the technique C++ forced you to propose. – user997112 Aug 14 '18 at 22:54

Iterate over container in either direction AND erase elements

1 Answers1