Search code examples
c++dictionaryfor-loopiterator

Is it okay to assign values to map.end()?


I'm wondering whether it's a bad practice to assign values 1 past the last element in a map like the example below.

using namespace std;

auto chances = map<int, int>{};
chances[0] = 20;
chances[1] = 10;
chances[2] = 30;
int last = 0;
for (auto it = chances.begin(); it != chances.end();) {
    last = it->second;
    (++it)->second += last;
}

Also, in a for loop is it faster to check against a variable than a function for termination (what is this part of the loop called?)


Solution

  • Yes, it's bad practice to assign to the end() iterator of any container (not just map)

    In all standard C++ containers, the end() iterator is not dereferencable. Any attempts to dereference (in this case, assign) to the end() iterator is undefined behavior.

    In your example code, a dereference of this end() iterator occurs due to the pre-increment operator being used:

    (++it)->second += last
    

    When it is 1 before end() during the iteration, this will increment it and dereference the result (the end) for the assignment.


    Also, in a for loop is it faster to check against a variable than a function for termination

    Generally it's better to assign the termination condition to a constant first and compare against that. Although compilers can perform this transformation, there are a number of factors that may result in the function call being repeatedly evaluated each iteration.

    That said, benchmark for yourself, and don't prematurely optimize. Small things like this seldomly make big differences unless they are in a tight loop.

    Note: Please try to ask only 1 question per SO post to help for searchability and prevent it from being closed for being too broad.


    Oh and yes, I'm using namespace std; ;)

    You should train yourself not to because it's bad practice that only exists due to legacy. Plus your future coworkers will thank you.