CODEWITHSUNDEEP
c++visual-studiocompiler-errorsstdmap
s_s
s_s

Reputation: 121

Compilation error in find in a std::map of string, long

I am working on Visual Studio 2013 in VC++. I have a std::map as follows,

std::map<std::string, unsigned int> myMap;

as a static member of a class.

I populate values into it by doing this,

string key = "value";
std::map<std::string, unsigned int>::iterator it = myMap.find(key);

if(it == myMap.end())
    myMap.insert(std::pair<std::string, unsigned int> (key, 1));
else
{
    int prev_value = it->second;
    prev_value++;
    myMap.insert(std::pair<std::string, unsigned int> (key, prev_value));
}

This does not compile and I get this compilation error,

1   IntelliSense: no suitable user-defined conversion from "std::_Tree_iterator<std::_Tree_val<std::_Tree_simple_types<std::pair<const std::string, long>>>>" to "std::_Tree_iterator<std::_Tree_val<std::_Tree_simple_types<std::pair<const std::string, unsigned int>>>>" exists  c:\filename.cpp 15

15 is this line,

std::map<std::string, unsigned int>::iterator it = myMap.find(key);

and this error,

2   IntelliSense: no operator "==" matches these operands
        operand types are: std::_Tree_iterator<std::_Tree_val<std::_Tree_simple_types<std::pair<const std::string, unsigned int>>>> == std::_Tree_iterator<std::_Tree_val<std::_Tree_simple_types<std::pair<const std::string, long>>>> c:\filename.cpp  17

line 17 is this,

if(it == myMap.end())

Can somebody help me to solve this issue?

Thanks

Upvotes: 1

Views: 1671

Answers (1)

Jonathan Mee
Jonathan Mee

Reputation: 38939

You wouldn't be getting this error if your declaration were actually:

std::map<std::string, unsigned int> myMap

You would get this exact error however if your definition were perchance:

std::map<std::string, long> myMap

In his famous Almost Always Auto, Herb Sutter postulates the guideline:

Remember that preferring auto variables is motivated primarily by correctness, performance, maintainability, and robustness—and only lastly about typing convenience.

As Herb Sutter suggests auto (and make_pair) would not only make your code correct, but also make it more robust, that is it would support myMap independent of the declaration of which of the above declarations you used:

const string key = "value"s;
const auto it = arg.find(key);

if(it == arg.end()) {
    arg.insert(make_pair(key, 1));
} else {
    int prev_value = it->second;
    prev_value++;
    arg.insert(make_pair(key, prev_value));
}

Live Example

EDIT:

Igor Tandetinik has reminded me that map::insert:

Inserts element(s) into the container, if the container doesn't already contain an element with an equivalent key

Meaning that your entire else-block doesn't do anything, cause it only fires when the map does already contain an element with an equivalent key. Presumably you intended to use map::insert_or_assign or similar, in which case your entire code should simply be replaced by:

++myMap["value"s]

EDIT2:

Note that the above works because in C++ the subscript operator has higher precedence than the prefix increment operator. But myMap["value"s]++ will not work because the postfix increment has higher precedence than the subscript operator. See the C++ operator precedence chart for more information on operator precedence.

And this ordering is a good thing in this case as it forces you to better coding decisions. Specifically, in C++ best practice is to use the prefix increment operator not the postfix increment operator for optimization reasons (though in most cases modern compilers will clean this up for you the best practice still stands.)

Upvotes: 1

Related Questions