9

This is similar to this question but not a duplicate. I'm trying to iterate through a map and print the values of each element, but with slightly different output on the last element. In that question, they recommend using map.rbegin().base(), but it's not working for me.

Here is my code:

#include <iostream>
#include <map>

int main()
{
    std::map <char, int> charMap = { {'a', 1}, {'b', 2}, {'c', 3}, {'d', 4} };
    for (auto iter = charMap.begin(); iter != charMap.end(); iter++)
    {
        std::cout << iter->first << ":\t" << iter->second;
        if (iter == charMap.rbegin().base())
            std::cout << "\t//This is the last element.\n";
        else
            std::cout << "\n";
    }
}

I'd expect my output to look like this:

a:    1
b:    2
c:    3
d:    4    //This is the last element. 

But instead, I am getting this output:

a:    1
b:    2
c:    3
d:    4    

Now I realize that there is a better way to do this, but I would expect this to work also. Why can't I compare iter and charMap.rbegin().base()?

Community
  • 1
  • 1
DJMcMayhem
  • 7,285
  • 4
  • 41
  • 61
  • Note that in such situations it's often easy to rewrite the loop body such that the special handling occurs for the first rather than the last element. – Christian Hackl Nov 11 '15 at 19:09
  • 1
    @ChristianHackl Note, that obviously this is not the case here :) – BartoszKP Nov 11 '15 at 19:27
  • 1
    @BartoszKP: Why not? Start every line but the first with a line break, put the "This is the last element" after the loop. – Christian Hackl Nov 11 '15 at 19:29
  • 1
    @ChristianHackl You're right, however OP does realize that, and question is specifically targeted for the flow as it is. – BartoszKP Nov 11 '15 at 19:45

7 Answers7

16

Use std::next from <iterator> as

if (std::next(iter) == charMap.end())
    std::cout << "\t//This is the last element.\n";

instead.

vsoftco
  • 55,410
  • 12
  • 139
  • 252
5

base() method returns iterator to an element that follows the element to which a reverse iterator is pointing. This means that rbegin().base() is equivalent to end()

To accomplish your task, you can do this:

#include <iostream>
#include <map>

int main()
{
    std::map <char, int> charMap = { {'a', 1}, {'b', 2}, {'c', 3}, {'d', 4} };
    for (auto iter = charMap.begin(); iter != charMap.end(); )
    {
        std::cout << iter->first << ":\t" << iter->second;
        if (++iter == charMap.end())
            std::cout << "\t//This is the last element.\n";
        else
            std::cout << "\n";
    }
}
AlexStepanov
  • 451
  • 3
  • 13
4

Because std::reverse_iterator stores iterator with offset of 1, so that charMap.rbegin().base() == charMap.end(). Chart over here illustrates this.

You should instead use std::prev or std::next from <iterator>:

iter == std::prev(charMap.end())

or

std::next(iter) == charMap.end()

Whichever you find more meaningful.

yuri kilochek
  • 12,709
  • 2
  • 32
  • 59
2

base() doesn't return the same element:

The base iterator refers to the element that is next (from the std::reverse_iterator::iterator_type perspective) to the element the reverse_iterator is currently pointing to. That is &*(rit.base() - 1) == &*rit.

This works as intended:

#include <iostream>
#include <map>

int main()
{
    std::map <char, int> charMap = { {'a', 1}, {'b', 2}, {'c', 3}, {'d', 4} };
    auto last = charMap.rbegin();
    ++last;

    for (auto iter = charMap.begin(); iter != charMap.end(); iter++)
    {
        std::cout << iter->first << ":\t" << iter->second;
        if (iter == last.base())
            std::cout << "\t//This is the last element.\n";
        else
            std::cout << "\n";
    }
}
BartoszKP
  • 34,786
  • 15
  • 102
  • 130
1

Actually, this is not expect to work as iter is indeed different from charMap.rbegin().base().

Your logic is correct, the only problem is that charMap.rbegin().base() iterator is equal to charMap.end(), but you leave your for statement at the moment iter reaches the charMap.end() value, so it never got the chance to test it in the if statement inside the for loop.

To make it work you could rewrite the if statement in the following way:

if (iter->first == charMap.rbegin()->first)
    std::cout << "\t//This is the last element.\n";
else
    std::cout << "\n";

So you will be comparing the map key instead of the iterator itself.

0

Ugly . . . but . . .

    auto iterCpy = iter;
    if (++iterCpy == charMap.end())
        std::cout << "\t//This is the last element.\n";

use vsoftco's answer :)

learnvst
  • 15,455
  • 16
  • 74
  • 121
0

This is working:

#include <iostream>
#include <map>

int main()
{
    std::map <char, int> charMap = { {'a', 1}, {'b', 2}, {'c', 3}, {'d', 4} };
    for (auto iter = charMap.begin(); ;)
    {
        std::cout << iter->first << ":\t" << iter->second;          
        if (++iter== charMap.end()){
            std::cout << "\t//This is the last element.\n"; break;
        }
        else
            std::cout << "\n";
    }

}
Ikbel
  • 7,721
  • 3
  • 34
  • 45