std::map - erase last element

Question

My map is defined as such: map<string, LocationStruct> myLocations; where the key is a time string

I am only keeping 40 items in this map, and would like to drop off the last item in the map when i reach 40 items. I know that i can't do myLocations.erase(myLocations.end()), so how do i go about this?

I do intend for the last item in the map to be the oldest, and therefore FIFO. The data will be coming in rather quick (about 20Hz), so i'm hoping that the map can keep up with it. I do need to look up the data based on time, so i really do need it to be the key, but i am open to alternate methods of accomplishing this.

The format of the string is a very verbose "Thursday June 21 18:44:21:281", though i can pare that down to be the seconds since epoch for simplicity. It was my first go at it, and didn't think too much about the format yet.

Would `myLocations.erase(myLocations.rbegin())` not do the job? — Rook, Jun 21 '12 at 17:12
It sounds like, instead of a (sorted) map, you'd like some sort of fixed-length queue / priority-queue. — kennytm, Jun 21 '12 at 17:13
@Rook `map<>::erase` takes an iterator, and not a reverse_iterator, as its position argument. — James Kanze, Jun 21 '12 at 17:26
You should strive to treat date/times as a number in your program's logic, and only use text format for input/output purposes. — Emile Cormier, Jun 21 '12 at 18:49
If you want to erase the last element you can simply do: `map::iterator it = myLocations.end(); it--;` and then erase `it`; — Rontogiannis Aristofanis, Jun 23 '12 at 14:53
Your date format is quite inconvenient. A `time_t` type seconds format would be significantly easier to work with. If you must use a string format for whatever reason, use a (relatively) sane and standardised system like ISO 8601, in which temporal and lexicographical sort orders would be the same. — Rook, Jun 24 '12 at 09:34

score 18 · Answer 1 · answered Jun 21 '12 at 17:32

18

The most idiomatic way would be:

myLocations.erase( std::prev( myLocations.end() ) );

If you don't ha ve C++11, use the corresponding function from your toolbox.

answered Jun 21 '12 at 17:32

James Kanze

150,581
18
184
329

That assumes the strings are sorted chonologically rather than lexicographically – Mooing Duck Jun 21 '12 at 18:02
3

@MooingDuck That's what he asked for: how to remove the last element. – James Kanze Jun 22 '12 at 07:55
The question has been updated to clarify: "I do intend for the last item in the map to be the oldest, and therefore FIFO." "The format of the string is a very verbose 'Thursday June 21 18:44:21:281'... ", given that information, this answer is wrong. – Mooing Duck Jun 22 '12 at 15:44
1

@MooingDuck You seem to be contradicting yourself. If the last item in the map is the oldest, then the above is correct. The format of the string is really irrelevant; it's up to the comparison function of the map to handle it correctly (although since he's worrying about speed, I'd index using a time stamp of some kind, rather than a string). – James Kanze Jun 22 '12 at 17:34
You seem to be suggesting he use a different container type then. If so, that should be in the answer, not the comments. `map`, with a description of how to write the comparitor. – Mooing Duck Jun 22 '12 at 17:42
1

He was quite clear with regards to the desired sort criterion. And whether he achieves this by overloading `<` on `LocationStruct`, by explicitly specializating `std::less`, or by defining a separate ordering function or functional object really doesn't affect the issue. – James Kanze Jun 25 '12 at 07:53
He's also clear that the type is `map myLocations;`, so he has no custom predicate, and since it's sorted on `string`, he can't overload `operator<` either. He _could_ have overloaded `std::less`, but that seems dangerous and unlikely. – Mooing Duck Jun 25 '12 at 15:44

score 8 · Answer 2 · answered Jan 25 '14 at 09:05

8

Try this, it works:

map<string, LocationStruct>::iterator it = myLocations.end();
it--;
myLocations.erase(it);

answered Jan 25 '14 at 09:05

Ahmed

631
7
5

unless `myLocations` has a size of zero, but it is probably an assumption that this is given – IceFire Jun 04 '16 at 20:26

Mooing Duck · Accepted Answer · 2012-06-21T23:52:11.903

5

I assume when you say "erase last element", you mean "erase oldest element".

I wouldn't use a string for times, use a date/time type instead (like unix timestamp). Then they'll be sorted by time, instead of lexicographically, and you can myLocations.erase(myLocations.begin()), since the oldest would always be at the beginning.

Even better, use a boost::circular_buffer<std::pair<timetype, LocationStruct>>, and use std::lower_bound to find elements by time. This will automatically remove the oldest for you, and has the same logorithmic complexity on finding an element by time. It's also faster when adding data. It's pretty much win all around for your situation. If you really want to avoid boost, then a std::deque fits your needs best, and gives great performance, but if you already have a working map, then staying with a std::map is probably best.

Here's how to do the find in a deque:

typedef ???? timetype;
typedef std::pair<Timetype, LocationStruct> TimeLocPair
typedef std::deque<TimeLocPair> LocationContainer;
typedef LocationContainer::const_iterator LocationIterator;

bool compareTimeLocPair(const TimeLocPair& lhs, const TimeLocPair& rhs)
{return lhs.first < rhs.first;}

LocationIterator find(const LocationContainer& cont, timetype time) {
    TimeLocPair finder(time, LocationStruct());
    LocationIterator it = std::lower_bound(cont.begin(), cont.end(), finder, compareTimeLocPair);
    if (it == cont.end() || it->first != time)
        return cont.end();
    return it;
}

edited Jun 21 '12 at 23:52

answered Jun 21 '12 at 17:14

Mooing Duck

64,318
19
100
158

It depends on how the string is being stored for date It had to take care of date if say the time doesnot belong to same day , Which i have mentioned in my answer – Invictus Jun 21 '12 at 17:17
@Ritesh: That is what I intended, but as per your confusion, I have clarified the words and provided a sample type. – Mooing Duck Jun 21 '12 at 17:21
I will have to investigate exactly how i will receive the time in order to find the key. That will drive my key format. – Jason Jun 21 '12 at 17:51
I'm not exactly thrilled about the thought of having to import more boost libraries into my project. Last time i did regex stuff, i think it added 50-something and that was even after i pared it down. – Jason Jun 21 '12 at 18:13
1

@Jason, if you can't or don't want to use Boost, a `std::deque` will work as a substitute for `boost::circular_buffer`. Just delete elements at the end when the FIFO reaches the maximum size. Also, be aware that `boost::circular_buffer` is a header-only library and I doubt it will increase the size of your executable that much. Instantiating a `boost::circular_buffer` template will probably add as much bloat as instantiating a `std::deque`. – Emile Cormier Jun 21 '12 at 18:15
1

To help you decide, here is the Rationale section of `circular_buffer`'s documentation: http://www.boost.org/doc/libs/release/libs/circular_buffer/doc/circular_buffer.html#rationale – Emile Cormier Jun 21 '12 at 18:21
@EmileCormier: I'm not sure a `std::deque` is going to help much over the `std::map`, especially since he already has a `std::map`. – Mooing Duck Jun 21 '12 at 18:21
@MooingDuck, if I'm not mistaken, inserting at the end of `deque` is amortized O(1), whereas inserting at the end of a `map` is O(log(N)) (unless one uses the `insert` overload with the "hint" argument). – Emile Cormier Jun 21 '12 at 18:26
@EmileCormier: yes, but when N is always 40, that difference is... minescule. And he _already_ has a map. – Mooing Duck Jun 21 '12 at 18:29
2

@MooingDuck: I suppose switching to a `deque` could be considered premature optimization if he already has it almost working with `map`. But deep inside, it bothers me that a `map` is used to implement a FIFO data structure instead of a `deque`. :-) – Emile Cormier Jun 21 '12 at 18:33
I'm not terribly dead set on a map. the 2 criteria i have are these: I need to be able to search on a unique key, and i need to keep 40 (an arbitrary, but realistic number) of my most recent locations. I opened it up to say that if there are more appropriate solutions, then i'm happy to look. – Jason Jun 21 '12 at 18:51
@Jason: A `deque` would be the better choice, but if you already have a `map`, it may or may not be worth changing. I'd recommend changing to a `deque` _if_ it's viable. – Mooing Duck Jun 21 '12 at 18:55
@MooingDuck - i really have very little code written for this, so it isn't a detriment to change it. I will read up and try implementing it. – Jason Jun 21 '12 at 19:04
I'm not seeing anything relating to a find on the deque. it only allows access of data via the index, which does not fit in with my requirements. – Jason Jun 21 '12 at 19:06
@Jason: Right, that's what `std::lower_bound` is for. See my edit. It has the same complexity as using a map, so no worries there. – Mooing Duck Jun 21 '12 at 19:12

score 2 · Answer 4 · answered Jun 21 '12 at 17:17

2

Well, a quick check on g++ 4.4 suggests that this works just fine:

myLocations.erase(myLocations.rbegin()->first);

though I must confess I don't know why it doesn't like accepting only the iterator itself.

answered Jun 21 '12 at 17:17

Rook

5,734
3
34
43

That passes the key of the last element, and a key is a valid thing to pass to `erase`. This is slow though, compared to simply passing an iterator to the last element. – Mooing Duck Jun 21 '12 at 17:18
1

Because `rbegin` returns a _reverse_ iterator – K-ballo Jun 21 '12 at 17:19
2

Yes, I am aware of that. Hence why I actually said _"I don't know why it doesn't like accepting only the iterator itself"_. – Rook Jun 21 '12 at 17:20
1

There's no overload of `erase()` that takes `reverse_iterator`, that's why. – jrok Jun 21 '12 at 17:20
1

@K-ballo: is there any technical reason why a reverse-iterator is not an appropriate thing to pass to `erase`, though? – Rook Jun 21 '12 at 17:20
1

Also, curious to know why a valid and syntactically correct answer warrants a downvote. The performance overhead of deferencing the iterator is not vast, after all. – Rook Jun 21 '12 at 17:22
1

@Rook: Not technically, since something like `++rbegin().base()` should return the corresponding regular iterator (or was it `--`?). – K-ballo Jun 21 '12 at 17:22
@Rook Only that there is no corresponding function overload. I would guess that the authors of the library didn't think it was generally useful enough. – James Kanze Jun 21 '12 at 17:29
@K-ballo You can't reliably use `++` or `--` on a temporary, so your solution won't work. (And how is it different from `--end()`---`rbegin().base()` is guaranteed to return `end()`. – James Kanze Jun 21 '12 at 17:35
Yes, it is valid. yes, it it is syntactically correct. Downvoted because it works the same as other answers, but is harder to read and far slower. – Mooing Duck Jun 21 '12 at 18:04
@Rook: I'm confident a map can delete by iterator faster than doing a full key search followed by a delete by iterator. Also your answer (and the others) makes no mention of needing a custom comparitor to make it work. – Mooing Duck Jun 22 '12 at 17:50
It is slower, yes, but in such a simple scenario the performance difference will be effectively unnoticeable. The performance requested by the OP is ~20hz, well within O(Log(N)) for a 40-element map. Your custom comparitor point is entirely correct however; my bad. I'll drop a comment on the original post suggesting a sane date/time format. – Rook Jun 24 '12 at 09:32

Invictus · Answer 5 · 2012-06-22T20:57:59.323

0

Since you are Storing the time as key String . The last element (earliest by time in a day considering time from 00:00 to 24:00) will be a lower bound element and hence You can fetch the iterator like this

     `map<string, LocationStruct>::iterator it;`
      it=myLocations.lower_bound ('00:00');
      myLocations.erase ( it, it+1);

But if it belongs to different dates then you need to even consider the day and manupilate your code accordingly . As you mentioned data is coming quick enough you dont need to take the date into consideration . But The safe way here would be take the entire date in terms of second and remove the lowest one as mentioned above . That would take care even if frequency of new data arriving is pretty slow.

edited Jun 22 '12 at 20:57

answered Jun 21 '12 at 17:16

Invictus

4,028
10
50
80

still doesnt handle 23:59 followed by 00:01 – Mooing Duck Jun 23 '12 at 05:43
@MooingDuck That is Why i mentioned The safe way here would be take the entire date in terms of second and remove the lowest one as mentioned above – Invictus Jun 23 '12 at 06:54
rereading your answer, you are correct, but the sentance before seems to contradict that advice. If you could clarify your wording (and comment so I notice), I'll remove the downvote. – Mooing Duck Jun 23 '12 at 17:00

score 0 · Answer 6 · edited May 23 '19 at 14:25

0

map::erase for last element without TS bindings, simply use following:

myLocations.erase ((--myLocations.end()));

edited May 23 '19 at 14:25

double-beep

5,031
17
33
41

answered May 23 '19 at 14:24

Jared Denisov

1

std::map - erase last element

6 Answers6