-1

I have a simple database consisting of objects with strings containing unix time as keys and strings containing instructions as values

I want to iterate though the database and erase any object who's key is smaller that current time ( so erase objects with dates before current date)

for (auto it = m_jsonData.begin(); it != m_jsonData.end(); it++) {
    if (std::stoi(it.key()) <= (std::time(NULL))) {
    std::cout << "command is behind schedule, removing\n";
    m_jsonData.erase(it);
    } else {
    /*
    */
    }
}

this code works fine as long as m_jsonData.erase(it); isn't invoked. when it does, in the next iteration std::stoi(it.key()) causes a segfault, after a bit of playing with it I came to a conclusion that is somehow loses track of what it's actually iterating. Is my conclusion true? If not then what is? And how do I fix it?

Serfer
  • 3
  • 1
  • Without knowing the exact type of `m_jsonData` I 'd suspect that `erase(it)` will invalidate the iterators. – πάντα ῥεῖ May 10 '22 at 10:36
  • @πάνταῥεῖ m_jsonData is a basic_json data type as described here [link](https://json.nlohmann.me/api/basic_json/basic_json/) (internally is a some form of associative container) From what I understand removing elements in a iteration range is a defined behaviour as shown in examples here [link](https://json.nlohmann.me/api/basic_json/erase/) – Serfer May 10 '22 at 10:43
  • Usually you'll get the next valid iterator back as result from `erase()`, the problem is the `it++` at the next loop iteration. – πάντα ῥεῖ May 10 '22 at 10:45

1 Answers1

2

It's extremely normal for mutating container operations to invalidate iterators. It's one of the first things you should check for.

Documentation for nlohnmann::json::erase():

Notes

  1. Invalidates iterators and references at or after the point of the erase, including the end() iterator.
  1. References and iterators to the erased elements are invalidated. Other references and iterators are not affected.

That means after this line:

m_jsonData.erase(it);

the iterator it can't be used for anything including incrementing it to the next element. It is invalid.

Fortunately, the documentation also points out that the successor to the removed element is returned, so you can just write

for (auto it = m_jsonData.begin(); it != m_jsonData.end(); ) {
    if (std::stoi(it.key()) <= (std::time(NULL))) {
        it = m_jsonData.erase(it);
    } else {
        ++it;
    }
}

Note that when I say this is extremely normal, it's because the standard containers often have similar behaviour. See the documentation for examples, but this is something everyone should be aware of:

This is exactly the reason std::erase was added in C++20, and previously std::remove_if was provided to suppport the erase(remove_if(...), end) idiom, instead of writing fragile mutating loops.

Useless
  • 64,155
  • 6
  • 88
  • 132
  • Yep, that works, thank you very much, turns out learning reading comprehension is a life-long process! – Serfer May 10 '22 at 10:49
  • 1
    If `std::vector::erase` was mentioned then it is worth pointing out that C++20 has introduced [std::erease](https://en.cppreference.com/w/cpp/container/vector/erase2) which helps remove items from a vector. – Marek R May 10 '22 at 11:44
  • Good point, I'll add a note. – Useless May 10 '22 at 12:00