58

I have a std::set<Foo>, and I'd like to update some value of an existing element therein. Note that the value I'm updating does not change the order in the set:

#include <iostream>
#include <set>
#include <utility>

struct Foo {
  Foo(int i, int j) : id(i), val(j) {}
  int id;
  int val;
  bool operator<(const Foo& other) const {
    return id < other.id;
  }
};

typedef std::set<Foo> Set;

void update(Set& s, Foo f) {
  std::pair<Set::iterator, bool> p = s.insert(f);
  bool alreadyThere = p.second;
  if (alreadyThere)
    p.first->val += f.val; // error: assignment of data-member
                           // ‘Foo::val’ in read-only structure
}

int main(int argc, char** argv){
  Set s;
  update(s, Foo(1, 10));
  update(s, Foo(1, 5));
  // Now there should be one Foo object with val==15 in the set.                                                                
  return 0;
}

Is there any concise way to do this? Or do I have to check if the element is already there, and if so, remove it, add the value and re-insert?

Frank
  • 64,140
  • 93
  • 237
  • 324
  • http://stackoverflow.com/questions/2217878/c-stl-set-update-is-tedious-i-cant-change-an-element-in-place – Ciro Santilli OurBigBook.com Jan 09 '17 at 16:19
  • Possible duplicate of [C++ STL set update is tedious: I can't change an element in place](https://stackoverflow.com/questions/2217878/c-stl-set-update-is-tedious-i-cant-change-an-element-in-place) – underscore_d Sep 22 '18 at 12:39
  • 1
    A little off-topic correction: the `second` member of the pair returned by `insert()` has the opposite meaning. It is `true` when the insertion has actually taken place, and `false` when the element was already there. – Alex Che Sep 22 '21 at 12:25

6 Answers6

68

Since val is not involved in comparison, it could be declared mutable

struct Foo {
  Foo(int i, int j) : id(i), val(j) {}
  int id;
  mutable int val;
  bool operator<(const Foo& other) const {
    return id < other.id;
  }
};

This implies that the value of val may change in a logically-const Foo, which means that it shouldn't affect other comparison operators etc.

Or you could just remove and insert, that takes O(1) additional time (compared to accessing and modifying) if insertion uses the position just before just after the old one as the hint.

Something like:

bool alreadyThere = !p.second; // you forgot the !
if (alreadyThere)
{
    Set::iterator hint = p.first;
    hint++;
    s.erase(p.first);
    s.insert(hint, f);
}
Cubbi
  • 46,567
  • 13
  • 103
  • 169
  • According to the SGI STL implementation, the Amortized O(1) insert is called with the iterator that is just **after** the insertion point. You'll want to use the element after for the hint, which makes it easier, since if you're pointing at an element, it cannot be `end()` – Dave S Sep 07 '11 at 21:18
  • @Dave S: yes, C++ library requirements say the same thing, just edited. – Cubbi Sep 07 '11 at 21:21
  • Good answer, but what happens if hint == s.begin()? When you erase the item that an iterator points to from the set you will invalidate that iterator. You need to check for that and insert with s.begin() as the hint in that case. – A. Levy Sep 07 '11 at 21:50
  • 3
    That's interesting: according to my drafts of the C++03 and C++11 standards, the meaning of the hint was changed between the two versions: in C++03, the expression `a.insert(p,t)` (where `a` is an associative container, `p` a hint iterator and `t` the value to insert) must have a complexity `logarithmic in general, but amortized constant if t is inserted right *after* p`. This last sentence was modified in C++11, and now states that the complexity should be `amortized constant if t is inserted right *before* p`. (cf. table 69 (C++03) and table 102 (C++11)). – Luc Touraille May 31 '12 at 14:42
  • Like @DaveS said, the new version is much more useful, since it allows "updating" elements much more easily. – Luc Touraille May 31 '12 at 14:45
  • 2
    I just found [these comments](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2005/n1780.html), which explains very well why the previous version was a mistake. – Luc Touraille May 31 '12 at 14:51
  • @LucTouraille Does this mean there will be different behavior depending if the code is compiled as C++11 or not? Is this correct? – user362515 Apr 13 '16 at 21:45
  • @user362515: No, the behavior will be the same, it is only the complexity requirements that have changed. The insertion will be performed correctly both in C++03 and in C++11, but the time it takes to do the insertion may be a bit different (I doubt this would be the case on any compiler though). – Luc Touraille Apr 14 '16 at 07:27
  • @LucTouraille I meant internal behavior. The answer should be updated with your info. Thanks! – user362515 Apr 14 '16 at 20:29
  • 2
    If you see pattern like `std< { k; mutable v; } >` you definitely must use `map< k, v >` instead. `mutable` is dirty workaround. – Tomilov Anatoliy Oct 23 '16 at 15:21
30

Don't try to solve this problem by working around the const-ness of items in a set. Instead, why not use map, which already expresses the key-value relationship you are modeling and provides easy ways to update existing elements.

Mark B
  • 95,107
  • 10
  • 109
  • 188
  • True, choose the right container for the right problem. – austingae Jul 30 '18 at 23:08
  • Once in a while I encounter this design issues and still did not find a good solution. If my objects are simple, then I can use map to manage the objects. Say my objects uses 5 properties for ordering, I may have to copy them into a tuple as a key. In some situations, mutable strategy is very bad. If any one has a good idea please post here. – Kemin Zhou Jun 08 '20 at 16:41
  • @KeminZhou just use `Boost.MultiIndex` there you can have `std::set` like container and modify it. – Slava Apr 07 '22 at 17:57
10

Make val mutable as:

mutable int val;

Now you can change/modify/mutate val even if foo is const:

void f(const Foo & foo)
{
     foo.val = 10;  //ok
     foo.id  = 11;  //compilation error - id is not mutable.
}

By the way, from your code, you seem to think that if p.second is true, then the value already existed in the set, and therefore you update the associated value. I think, you got it wrong. It is in fact other way round. The doc at cpluscplus says,

The pair::second element in the pair is set to true if a new element was inserted or false if an element with the same value existed.

which is correct, in my opinion.


However, if you use std::map, your solution would be straightforward:

void update(std::map<int,int> & m, std::pair<int,int> value) 
{
    m[value.first] += value.second;
}

What does this code do? m[value.first] creates a new entry if the key doesn't exist in the map, and value of the new entry is default value of int which is zero. So it adds value.second to zero. Or else if the key exists, then it simply adds value.second to it. That is, the above code is equivalent to this:

void update(std::map<int,int> & m, std::pair<int,int> value) 
{
    std::map<int,int>::iterator it = m.find(value);
    if ( it != m.end()) //found or not?
           it.second += value; //add if found
    else
    {
           m.insert(value); //insert if not found
    }
}

But this is too much, isn't it? It's performance is not good. The earlier one is more concise and very performant.

Nawaz
  • 353,942
  • 115
  • 666
  • 851
1

If you know what you're doing (the set elements are not const per se and you're not changing members involved in comparison), then you can just cast away const-ness:

void update(Set& s, Foo f) {
  std::pair<Set::iterator, bool> p = s.insert(f);
  bool alreadyThere = !p.second;
  if (alreadyThere)
  {
    Foo & item = const_cast<Foo&>(*p.first);
    item.val += f.val;
  }
}
Alex Che
  • 6,659
  • 4
  • 44
  • 53
0

Instead of separate hint, use the erase's return value for the next iterator position

bool alreadyThere = !p.second; 
if (alreadyThere)
{
    auto nit = s.erase(p.first);
    s.insert(nit, f);
}
U J
  • 61
  • 1
  • 5
-4

you can use MAP witch has very fast access to your element if you have KEY . in this case i think using MAP would be better way to achieve fastest speed . STD::MAP

sam
  • 1,363
  • 1
  • 20
  • 32