Determine two strings if they are anagrams using unordered_map

Question

I know that many questions already pose this problem, but I want to do this using std::unordered_map in c++. If someone has some answer similar to my implementation please point me to some link. Thanks.

I have used hashing as a way to count occurrences of the characters in a string and using that function to compare two strings if they have same number of same characters but I think I may have some issue in returning the correct prompt.

#include<iostream>
#include<bits/stdc++.h>
#include<string>
using namespace std;

unordered_map<char,int> countOccurences(string s){
    unordered_map<char,int> m;
    for(int c=0;c<s.length();c++){ 
        if(s[c]!=' '){
            //for type-insensitive counting
            if((int)s[c] || (int)s[c] + 32){
                m[s[c]]++;
            }
        }
    }
    return m;
}
int main(){
    string s1,s2;
    cout<<"Enter two strings to be checked:\n";
    cin>>s1;
    cin>>s2;
    unordered_map<char,int> m1,m2;
    m1 = countOccurences(s1);
    m2 = countOccurences(s2);

    for(int i=0;i<=255;i++){
        if(m1[i]!=m2[i])
            cout<<"Strings are not anagrams\n";
            break;
            return 0;
    }
    cout<<"Strings match\n";
    return 0;
}

I expect output to be showing "Strings are not anagrams" but I am getting "Strings match"

`unordered_map` has a `==` operator that works as one would expect. — molbdnilo, Jun 04 '19 at 08:52
[Why should I not #include ?](https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h) and [Why is “using namespace std;” considered bad practice?](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) — Stack Danny, Jun 04 '19 at 08:52
Note : you did not use curly braces in the `if` statement. Hence, you will never reach the first `return 0`. Hence, you will always reach `cout<<"Strings match\n";` — Arnaud Denoyelle, Jun 04 '19 at 08:53
Your inner conditional in the counting function is a bit strange - the conditions are always true. If you want to ignore case, count `std::toupper(s[c])` or `std::tolower(s[c])`. — molbdnilo, Jun 04 '19 at 08:54
I am not a C++ developer but is it legal tu return an `unordered_map`? Wouldn't it be better to pass the map to the function with a const ref ? It seems to me that doing it like this is bad for memory management (implicit call to constructor by copy ?). — Arnaud Denoyelle, Jun 04 '19 at 09:14
@ArnaudDenoyelle Pls give some more insight about it. I am new to this question. — harshrd, Jun 04 '19 at 09:22
@ArnaudDenoyelle Yes, it is legal. See e.g. https://en.cppreference.com/w/cpp/language/copy_elision — Bob__, Jun 04 '19 at 09:25
Worth asking for a review https://codereview.stackexchange.com (once it is working) — Martin York, Jun 04 '19 at 09:45

Arnaud Denoyelle · Accepted Answer · 2019-06-04T09:11:31.380

1

You are missing curly braces on the if statement here :

    if(m1[i]!=m2[i])
        cout<<"Strings are not anagrams\n";
        break;
        return 0;

So the code is equivalent to :

for(int i=0;i<=255;i++){
    if(m1[i]!=m2[i]) {
        cout<<"Strings are not anagrams\n";
    }
    break;
    return 0;
}

After the first comparison, the break statement is always reached, exiting the for loop.

You should replace it with :

for(int i=0;i<=255;i++){
    if(m1[i]!=m2[i]) {
        cout<<"Strings are not anagrams\n";
        return 0;
    }
}

Also, as stated in the question's comments, the case-sensitive test is weird, as if((int)s[c] will always be true.

You might want to write it like this instead :

for(int c=0;c<s.length();c++){ 
    if(s[c]!=' '){
        m[std::toupper(s[c])]++;
    }
}

edited Jun 04 '19 at 09:11

answered Jun 04 '19 at 08:51

Arnaud Denoyelle

29,980
16
92
148

Shouldn't `std::toupper(s[c])` be used to ignore case sensitivity. I mean does 'o' and 'O' mean the same in an Anagram? _Just Curious, I don't really know_ :P – RC0993 Jun 04 '19 at 09:29
Please also note that [`std::unordered_map::operator[]`](https://en.cppreference.com/w/cpp/container/unordered_map/operator_at) *"Returns a reference to the value that is mapped to a key equivalent to key, **performing an insertion if such key does not already exist.**"*. See e.g. [here](https://wandbox.org/permlink/xBW5KGh93t0A1iCn). Just use `==` as commented by molbdnilo. – Bob__ Jun 04 '19 at 09:49
@RC0993 Anagramness traditionally does not care about case - or whitespace or punctuation, for that matter. Hence "Madam, I'm Adam" and "A man, a plan, a canal - Panama!". – molbdnilo Jun 04 '19 at 10:27

Determine two strings if they are anagrams using unordered_map

1 Answers1