3

The following is C++ code to get a count of the words in magazine. I'm trying to add the word if its value does not exist and if it does, increment it. 

unordered_map<string,int>hash;
vector<string> magazine(m);

for(int i = 0;i <m;i++)
{
   cin >> magazine[i];
   if(hash[magazine[i]]>0)
       hash[magazine[i]]++;
   else
    hash.emplace(magazine[i],1);
}

But when I try to output, all magazine key gives 0 as value. Any idea why?

Saurav Sahu
  • 13,038
  • 6
  • 64
  • 79
Bharg
  • 311
  • 1
  • 2
  • 15

3 Answers3

8

Your version doesn't work because this if(hash[magazine[i]]>0) will insert an element into hash if it doesn't exist, this new element will have a mapped value of 0¹. Which means that hash.emplace(magazine[i],1); is pointless here because there will always be an element at magazine[i] now. Because its value will be 0 your hash[magazine[i]]++; will never run either because the if will never be true. Leaving you with a map of i elements, all with value 0.

operator[] returns a reference to the mapped value if there is one, if not, it inserts one and then returns that reference¹.

Which means that you can factor out the if and just change it to:

for(int i = 0;i <m;i++)
{
   cin >> magazine[i];
   ++hash[magazine[i]];
}

Which basically means : "Get a reference to the mapped value for key magazine[i], if none is found, insert one and give me that one. Increment this reference."

¹: If insertion occurs the element is value-initialized. Because your mapped value type is int this will result into the mapped value being 0 after insertion.

Toby Speight
  • 27,591
  • 48
  • 66
  • 103
Hatted Rooster
  • 35,759
  • 6
  • 62
  • 122
  • 1
    Please add to the answer that the newly created value is default initialized which means it holds a value of 0 for numeric types. – Leon Oct 24 '16 at 07:32
  • This works, but you haven't explained why the previous version didn't work (which is what the OP actually asked). – Martin Bonner supports Monica Oct 24 '16 at 07:34
  • 1
    @Leon It is *value* initialized, which means it holds a value of 0 for numeric types. Default initialization of numeric types doesn't do anything. – user703016 Oct 24 '16 at 07:35
1

You are inadvertently creating a new element in the map by doing :

if(hash[magazine[i]]>0)

map<>::operator[] does insertion, value initialization(that value is zero in your case), and then returns the reference of value, all very discreetly.

As correctly suggested in many comments, the best way is:

hash[key]++

Read more.

Community
  • 1
  • 1
Saurav Sahu
  • 13,038
  • 6
  • 64
  • 79
1

if(hash[magazine[i]]>0) creates new item if the key is not exist.

What you really want is:

if(hash.find(magazine[i])!=hash.end())

As @juanchopanza mentioned, you do not need the branching. std::unordered_map::operator [] can handle it like this:

hash[magazine[i]]++;
Humam Helfawi
  • 19,566
  • 15
  • 85
  • 160
  • 1
    No, they don't need any conditionals. Just `hash[magazine[i]]++;`. – juanchopanza Oct 24 '16 at 07:20
  • 1
    Or remove the `if` entirely, and *just* use `hash[magazine[i]]++;`. The element will be created with value 0 if necessary, and then incremented. This *is* the use-case for indexing into a map creating an element if it doesn't exist - I still thing it was the wrong decision though. – Martin Bonner supports Monica Oct 24 '16 at 07:22
  • 1
    @juanchopanza You mean that it defaulted with 0 and ++ is responsible to make it 1 ? even so, the OP needs a clarification about what is happening and what is the exact alternative IMO – Humam Helfawi Oct 24 '16 at 07:22
  • 1
    Not really, `hash[magazine[i]]++;` is always the answer here. – Hatted Rooster Oct 24 '16 at 07:23
  • @HumamHelfawi Yes, good to point out what is wrong. I tried to get them to figure it out by asking a question in the comments. – juanchopanza Oct 24 '16 at 07:24
  • I think you need to point out that .emplace will do nothing if the element already exists (and the test ensures it does) - thus the element will remain zero, and the next time round the loop, the same thing will happen. – Martin Bonner supports Monica Oct 24 '16 at 07:24
  • 1
    @juanchopanza @Martin @GillBates `hash[magazine[i]]++;` doesn't answer the question that was asked by `OP`. It is (possibly) a useful comment to add to the answer . – Galik Oct 24 '16 at 07:32
  • @Galik: Agreed. Hence my comment about emplace doing nothing because the element exists. – Martin Bonner supports Monica Oct 24 '16 at 07:36