1

having:

std::map<const std::string,A > cache;

how would you insert into this container(duplicate attempt is possible):

cache.insert(std::make_pair(id,ps));

cache.insert(std::pair<std::string,A>(id,ps));

if(cache.find(id) == cache.end()){
 cache[id] = ps;
}

and why??(in terms of time and memory)

do you have a better solution?

Update: I am not using C++11

Update-2: OK, so far we realised that:

make_pair and pair<> are analogous.

insert and [ ](with or without ifchecking) will both invoke copy. So.. Is the competition between :

  1. insert
  2. [ ](withifchecking)
  3. [ ](withifchecking and swap)

which one would you prefer?

thanks again

Community
  • 1
  • 1
rahman
  • 4,820
  • 16
  • 52
  • 86

3 Answers3

6

I'd suggest emplace:

template< class... Args >
std::pair<iterator,bool> emplace( Args&&... args );

// usage:
cache.emplace(id, ps);

But, it depends a bit on whether you want a second insertion to

  • fail (as the above would, returning a std::pair<iterator,bool> in which .second indicates successful insertion or failure - you can ignore that if you don't care, as the only reason for failure is an existing key) or

  • overwrite the existing value (for which it's easiest and usually perfectly adequate to write cache[id] = ps;, but if you want to avoid default construction of an element pair then you can use find and then overwrite an existing or emplace a new element as necessary).

Occasionally you might want to consider marking one or both of the parameters as movable-from (i.e. you don't care what (valid) state the local variable is left in after the emplace returns, which may allow extra optimisations). You can see from the prototype above that the arguments are accepted using && which hints at capability to perform such optimnisations.

cache.emplace(std::move(id), std::move(ps));
Tony Delroy
  • 102,968
  • 15
  • 177
  • 252
  • In this case emplace will not do what it designed for. – Galimov Albert Aug 20 '14 at 09:22
  • @PSIAlt that's not true (given my understanding of what it's designed for, which is probably less than some people might hope for) but it's also vague enough that I've nothing meaningful to dispute... care to elaborate? – Tony Delroy Aug 20 '14 at 09:31
  • emplace receives argument list, which will be forwared to constructor for new objects. Because you give a `std::string` as a initializer list - objects will be created using default copy constructor, which is not most efficient in terms of cpu-memory. – Galimov Albert Aug 20 '14 at 09:38
  • @PSIAlt I thought you might mean that... the question doesn't say what the type of `id` is - it could be e.g. a `const char*` to a string literal in which case the `std::string` will be constructed once where needed - nor anything about the types of `A` and `ps`, but I agree it could be a consideration for some types, if there's any actual reason to construct complex variables before the `emplace` line itself. `std::move()` on the `emplace` parameters is a natural solution where a local copy needn't be retained. – Tony Delroy Aug 20 '14 at 09:49
  • Nice discussion, but... I didn't mean C++11 (updated the question accordingly) – rahman Aug 20 '14 at 11:54
5

The third one is clearly inferior in terms of time, since it requiers two lookups in the map when actually doing the insertion (one for find() and one for []). So stick to the insert() function.

Now the question is: std::make_pair() or std::pair<A, B>()? It's generally a good idea to follow the principle of DRY, which means you shouldn't repeat the types, so use std::make_pair().

If you have C++11, the best option is to use emplace():

cache.emplace(id, ps);

As a side note, it's pointless to type the map as std::map<const std::string, A>. Keys in a map are already immutable; it's more concise (and doesn't produce a WTF moment) to just use std::map<std::string, A>, and you'll be really hard pressed to find a case where those two differ.

Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
  • 1
    Depending on required behavior on duplicate keys, `[]` can be strictly better. Although given the `if` guard in question, this is probably moot. – Xarn Aug 20 '14 at 09:16
  • @Xarn `std::map` does not allow duplicate keys. And all 3 of the OP's examples leave the value untouched when the key is already present, so I assume that's the intended behaviour. Can `[]` be better in this situation? – Angew is no longer proud of SO Aug 20 '14 at 09:18
  • `std::map<...>::value_type(id,ps)` is much better than the `pair` and `make_pair` solutions. – Marc Glisse Aug 20 '14 at 09:49
4
cache.insert(std::make_pair(id,ps));

This will copy id and ps. If they have "heavy" copy constructors this will waste some time and memory.

cache.insert(std::pair<std::string,A>(id,ps));

Its indeed analog for make_pair

if(cache.find(id) == cache.end()){
 cache[id] = ps;
}

Its better to use without check: cache[id] = ps;. But, at first element insertion the "default" object of type decltype(ps) will be constructed. Then, its assigned (using operator=) to ps. If its heavy, this can be problem too. But, i think its preferred way if swap method is not available.

cache.emplace(id, ps);

This can look like zero-copy emplace, but its not. emplace receives constructor parameters, so default copy constructor will be called. Also its C++11.

I think most efficient way is

cache[id].swap(ps);

This should make minimum amount of copies, but drawback is your ps will be reset (or contain old value).

Galimov Albert
  • 7,269
  • 1
  • 24
  • 50
  • 1
    Actually I was looking for such an answer-comparing the copy operations between `insert` and `[ ]`. Does this mean, both `insert` and `[ ]` will do a copy anyway? does it mean they dont differ much? – rahman Aug 20 '14 at 11:39
  • 1
    @rahman this can depend on implementation. With `insert` it can be a copy constructor + assign operation. In case `[]` you know it will be only create empty+assign operation, which should be faster anyway. `swap` will just swap object guts and should have constant time. – Galimov Albert Aug 20 '14 at 12:47