How to implement the iterator in C++?

Question

I have a problem with using iterator on the following code? Does anyone know how to fix it?

using StringVec = std::vector<std::string>;
using StringIntMap = std::unordered_map<std::string, int>;
  
StringIntMap makeWordCounts(const StringVec& words) {
   StringIntMap wordcount_map;
    
    std::vector<std::string>::const_iterator iter = words.begin();
    while(iter != words.end()) { //error message appears here
        if(wordcount_map.count(iter)) {
            wordcount_map[iter] = 1;
        }else{
            int value = wordcount_map.at(iter);
            wordcount_map[iter] = value+1;
        }
    }
  return wordcount_map;
}

error message: no viable conversion from

 'std::vector<std::string>::const_iterator' (aka '__wrap_iter<const
  std::__1::basic_string<char> *>') to 'const
  std::__1::unordered_map<std::__1::basic_string<char>, int,
  std::__1::hash<std::__1::basic_string<char> >,
  std::__1::equal_to<std::__1::basic_string<char> >,
  std::__1::allocator<std::__1::pair<const std::__1::basic_string<char>,
  int> > >::key_type' (aka 'const std::__1::basic_string<char>')
    if(wordcount_map.count(iter)) {

Thank you for your help.

Answer 1

As other answers have explained, you are getting the compiler error because you are not dereferencing the iterator to access the std::string it is referring to.

I just want to add that even with that error fixed, your code still has logic mistakes in it:

• your loop is not incrementing the iterator at all, so it will run endlessly if the words vector is not empty.

• your loop body is implemented backwards. std::unordered_map::count() returns the number of elements that match the requested key. An if statement treats a numeric value of 0 as false, and any other numeric value as true. So, if count() returns > 0 indicating a given key does exist, you are updating that element with a value of 1, wiping out its previous value. And if count() returns 0 indicating a given key does not exist, you are calling std::unordered_map::at() with that same key, which fails and throws a std::out_of_range exception.

The corrected version would look like this:

using StringVec = std::vector<std::string>;
using StringIntMap = std::unordered_map<std::string, int>;
  
StringIntMap makeWordCounts(const StringVec& words) {
   StringIntMap wordcount_map;
    
    StringVec::const_iterator iter = words.begin();
    while (iter != words.end()) {
        if (wordcount_map.count(*iter)) {
            int value = wordcount_map.at(*iter);
            wordcount_map[*iter] = value + 1;
        }else{
            wordcount_map[*iter] = 1;
        }
        ++iter;
    }

    return wordcount_map;
}

However, this code needlessly complicated and inefficient. It can be greatly simplified to this:

using StringVec = std::vector<std::string>;
using StringIntMap = std::unordered_map<std::string, int>;
  
StringIntMap makeWordCounts(const StringVec& words) {
    StringIntMap wordcount_map;
    
    for(const auto &word : words) {
        wordcount_map[word]++;
    }

    return wordcount_map;
}

A range-based for loop will handle iterators for you. And std::unordered_map::operator[] returns a reference to an element's value, inserting and initializing a new element for you if the requested key does not exist yet.

Answer 2

Assuming you want to use the std::string element of the words vector that iter currently references as your index/key in wordcount_map, then you simply need to dereference iter with the * operator. Also, as it stands, your while loop doesn't make any modification to the iter variable; you probably need an increment (++) operator on it at the end of the loop:

using StringVec = std::vector<std::string>;
using StringIntMap = std::unordered_map<std::string, int>;

StringIntMap makeWordCounts(const StringVec& words)
{
    StringIntMap wordcount_map;

    std::vector<std::string>::const_iterator iter = words.begin();
    while (iter != words.end()) {
        if (wordcount_map.count(*iter)) { // Use * to dereference iter
            wordcount_map[*iter] = 1;     // ... and here
        }
        else {
            int value = wordcount_map.at(*iter); // ...
            wordcount_map[*iter] = value + 1;    // ...
        }
        ++iter; // Increment the iterator to move on to the next vector element
    }
    return wordcount_map;
}

However, rather than adding ++iter as a separate line in a while loop, you could just use a for loop instead:

    for (std::vector<std::string>::const_iterator iter = words.begin(); iter != words.end(); ++iter) {
        //...

Or, even simpler, don't use an explicit iterator at all; just use a 'range-based' for loop:

    for (auto str : words) {
        if (wordcount_map.count(str)) {
            wordcount_map[str] = 1;
        }
        else {
            int value = wordcount_map.at(str);
            wordcount_map[str] = value + 1;
        }
    }

Answer 3

Take a look at what function you're calling:

wordcount_map.count(iter)
              ^^^^^

Now, take a look at the type of the parameter:

size_type count( const Key& key ) const;
                 ^^^^^^^^^^

Notice that the parameter is expected to be a key of the map. Now, take a look at the key type of you map:

using StringIntMap = std::unordered_map<std::string, int>;
                                        ^^^^^^^^^^^

It is a string. And finally, take a look at the type of the argument that you attempt to pass into the function:

std::vector<std::string>::const_iterator iter = words.begin();
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Can you see the problem? You are passing an iterator into a function that expects a string. That is an entirely wrong type of object, and not even convertible to the expected type. That is why you get the error:

error message: no viable conversion from [iterator] to [string]

---

You haven't described what you're trying to do, but given that the iterator points to a string, and the function that you call expects a string, I would guess that you want to pass the string pointed by the iterator into the function. To access the pointed string, you need to indirect through the iterator. This can be achieved using the indirection operator:

*iter

---

P.S. If the loop is entered, it will never end (except if something is thrown out of it or process is terminated) because iter is never modified and thus there cannot be change to the end condition.

Also, the else branch seems to always throw (which would end the loop).

Answer 4

The error is not in the line you think it is. In the line after and for the rest of the code you forgot to dereference iter and you pass an iterator where a std::string is needed. Fix:

    if(wordcount_map.count(*iter)) {
        wordcount_map[*iter] = 1;
    }else{
        int value = wordcount_map.at(*iter);
        wordcount_map[*iter] = value+1;
    }