How to achieve better efficiency re-inserting into sets in C++

Question

I need to modify an object that has already been inserted into a set. This isn't trivial because the iterator in the pair returned from an insertion of a single object is a const iterator and does not allow modifications. So, my plan was that if an insert failed I could copy that object into a temporary variable, erase it from the set, modify it locally and then insert my modified version.

insertResult = mySet.insert(newPep);
    if( insertResult.second == false )
        modifySet(insertResult.first, newPep);

void modifySet(set<Peptide>::iterator someIter, Peptide::Peptide newPep) {
    Peptide tempPep = (*someIter);
    someSet.erase(someIter);
    // Modify tempPep - this does not modify the key
    someSet.insert(tempPep);            

}

This works, but I want to make my insert more efficient. I tried making another iterator and setting it equal to someIter in modifySet. Then after deleting someIter I would still have an iterator to that location in the set and I could use that as the insertion location.

void modifySet(set<Peptide>::iterator someIter, Peptide::Peptide newPep) {
    Peptide tempPep = (*someIter);
    anotherIter = someIter;
    someSet.erase(someIter);
    // Modify tempPep - this does not modify the key
    someSet.insert(anotherIter, tempPep);            

}

However, this results in a seg fault. I am hoping that someone can tell me why this insertion fails or suggest another way to modify an object that has already been inserted into a set.

The full source code can be viewed at github.

Answer 1

I agree with Peter that a map is probably a better model of what you are doing, specifically something like map<pep_key, Peptide::Peptide>, would let you do something like:

insertResult = myMap.insert(std::make_pair(newPep.keyField(), newPep));
if( insertResult.second == false )
    insertResult.first->second = newPep;  

To answer your question, the insert segfaults because erase invalidates an iterator, so inserting with it (or a copy of it) is analogous to dereferencing an invalid pointer. The only way I see to do what you want is with a const_cast

insertResult = mySet.insert(newPep);
if( insertResult.second == false )
    const_cast<Peptide::Peptide&>(*(insertResult.first)) = newPep;

the const_cast approach looks like it will work for what you are doing, but is generally a bad idea.