Return to Answer

In addition to the bug JS1 discovered, namely that you only check whether all elements of the first string have equal frequency in the second string, there's the question why you are building a complete histogram of both strings. Doing so is inefficient. Instead, increment for one string, and decrement for the other one. The result should be zero.

Your compiler hopefully at least memoizes the result of strlen for you, or eliminates even that superfluous extra traversal. If it doesn't, that's a factor of n for your algorithms complexity right there.

bool areAnagrams(const char* s1, const char* s2) {
 std::unordered_map map<char, int>;
 while(*s1)
 ++map[*s1++];
 while(*s2)
 --map[*s2++];
 for(auto&& e : map)
 if(e.second)
 return false;
 return true;
}

It's probably far more efficient to use a plain array of appropriate size on the stack than a complicated std::unordered_map with dynamic allocation. Just beware of plain chars signedness.

bool areAnagrams(const char* s1, const char* s2) {
 int map[UCHAR_MAX + 1] = {};
 while(*s1)
 ++map[(unsigned char)*s1++];
 while(*s2)
 --map[(unsigned char)*s2++];
 for(auto e : map)
 if(e)
 return false;
 return true;
}

Also, avoid using namespace std;. I don't have conclusive evidence you used it, but to be safe, here it is: Why is "using namespace std;" considered bad practice?

In addition to the bug JS1 discovered, namely that you only check whether all elements of the first string have equal frequency in the second string, there's the question why you are building a complete histogram of both strings. Doing so is inefficient. Instead, increment for one string, and decrement for the other one. The result should be zero.

Your compiler hopefully at least memoizes the result of strlen for you, or eliminates even that superfluous extra traversal. If it doesn't, that's a factor of n for your algorithms complexity right there.Shlemiel The Painter greets you.

bool areAnagrams(const char* s1, const char* s2) {
 std::unordered_map map<char, int>;
 while(*s1)
 ++map[*s1++];
 while(*s2)
 --map[*s2++];
 for(auto&& e : map)
 if(e.second)
 return false;
 return true;
}

It's probably far more efficient to use a plain array of appropriate size on the stack than a complicated std::unordered_map with dynamic allocation. Just beware of plain chars signedness.

bool areAnagrams(const char* s1, const char* s2) {
 int map[UCHAR_MAX + 1] = {};
 while(*s1)
 ++map[(unsigned char)*s1++];
 while(*s2)
 --map[(unsigned char)*s2++];
 for(auto e : map)
 if(e)
 return false;
 return true;
}

Also, avoid using namespace std;. I don't have conclusive evidence you used it, but to be safe, here it is: Why is "using namespace std;" considered bad practice?

In addition to the bug JS1 discovered, namely that you only check whether all elements of the first string have equal frequency in the second string, there's the question why you are building a complete histogram of both strings. Doing so is inefficient. Instead, increment for one string, and decrement for the other one. The result should be zero.

bool areAnagrams(const char* s1, const char* s2) {
 std::unordered_map map<char, int>;
 while(*s1)
 ++map[*s1++];
 while(*s2)
 --map[*s2++];
 for(auto&& e : map)
 if(e.second)
 return false;
 return true;
}

It's probably far more efficient to use a plain array of appropriate size on the stack than a complicated std::map with dynamic allocation. Just beware of plain chars signedness.

bool areAnagrams(const char* s1, const char* s2) {
 int map[UCHAR_MAX + 1] = {};
 while(*s1)
 ++map[(unsigned char)*s1++];
 while(*s2)
 --map[(unsigned char)*s2++];
 for(auto e : map)
 if(e)
 return false;
 return true;
}

Also, avoid using namespace std;. I don't have conclusive evidence you used it, but to be safe, here it is: Why is "using namespace std;" considered bad practice?

In addition to the bug JS1 discovered, namely that you only check whether all elements of the first string have equal frequency in the second string, there's the question why you are building a complete histogram of both strings. Doing so is inefficient. Instead, increment for one string, and decrement for the other one. The result should be zero.

Your compiler hopefully at least memoizes the result of strlen for you, or eliminates even that superfluous extra traversal. If it doesn't, that's a factor of n for your algorithms complexity right there.

bool areAnagrams(const char* s1, const char* s2) {
 std::unordered_map map<char, int>;
 while(*s1)
 ++map[*s1++];
 while(*s2)
 --map[*s2++];
 for(auto&& e : map)
 if(e.second)
 return false;
 return true;
}

It's probably far more efficient to use a plain array of appropriate size on the stack than a complicated std::unordered_map with dynamic allocation. Just beware of plain chars signedness.

bool areAnagrams(const char* s1, const char* s2) {
 int map[UCHAR_MAX + 1] = {};
 while(*s1)
 ++map[(unsigned char)*s1++];
 while(*s2)
 --map[(unsigned char)*s2++];
 for(auto e : map)
 if(e)
 return false;
 return true;
}

Also, avoid using namespace std;. I don't have conclusive evidence you used it, but to be safe, here it is: Why is "using namespace std;" considered bad practice?