CODEWITHSUNDEEP
c++iteratorc++14
LBaelish
LBaelish

Reputation: 699

iterator automatically dereferenced when pointing to integer

I noticed that when the else if statement is being executed in this code destination, despite not being dereferenced, seems to be interpreted as the actual value it is pointing to instead of the iterator. It's like it's being automatically dereferenced. The for loop executes as you would expect, but id like done to be set to true when the begin() and end() iterators are equal. destinations is a deque of integers, global to this function.

void removeDestination(int destinationFloor)
{
    bool done = false;
    while(!done && !destinations.empty())
    {
        for (auto destination = destinations.begin(); destination != destinations.end(); ++destination)
        {
            if(*destination == destinationFloor)
            {
                destinations.erase(destination);
                break;
            }
            else if(destination == destinations.end())
            {
                done = true;
            }
        }
    }

}

Thanks for the help.

Upvotes: 1

Views: 75

Answers (3)

Mark Ransom
Mark Ransom

Reputation: 308216

It's generally a bad idea to modify a sequence while you're iterating it, it can cause the iterators to be invalidated leading to undefined behavior.

For a deque the erase can cause invalidation of all iterators, including the one you're using for the loop.

Edit: as pointed out in the comments, you've already developed a strategy for dealing with the invalidated iterators. It's not terribly efficient, but it should work. Your problem is in not setting the done variable properly.

The key here is to realize that you're done if you've performed the entire for loop without breaking. So you assume you're done unless you break out of the for loop.

bool done = false;
while(!done && !destinations.empty())
{
    done = true;
    for (auto destination = destinations.begin(); destination != destinations.end(); ++destination)
    {
        if(*destination == destinationFloor)
        {
            destinations.erase(destination);
            done = false;
            break;
        }
    }
}

Upvotes: 3

Tim
Tim

Reputation: 1527

I noticed that when the else if statement is being executed in this code destination, despite not being dereferenced, seems to be interpreted as the actual value it is pointing to instead of the iterator.

Nope. if(*destination == destinationFloor) compares the value pointed to by the iterator to the value destinationFloor. Whereas, if(destination == destinations.end()) compares two iterators.

destinations is a deque of integers, global to this function.

Global variables? Not using the standard algorithms? I would encourage you to take a look at a modern C++ book. In particular, the code you have written is a well-solved problem. See the erase-remove idiom.

Upvotes: 2

user2357112
user2357112

Reputation: 281187

The problem has nothing to do with destination being spuriously dereferenced. The else if branch is never taken:

else if(destination == destinations.end())

because if destination reaches the end of destinations, then the loop condition:

for (auto destination = destinations.begin(); destination != destinations.end(); ++destination)
//                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

stops the loop before the else if could be entered.

Upvotes: 5

Related Questions