Return to Answer

Our histogram maps char to int, which could potentially overflow (undefined behaviour again). We could use an unsigned type (which has well-defined overflow behaviour), but consider that we're only interested in whether the value is even or odd - so we could use a map from char to bool.

Also, an unordered map is a pretty slow form of histogram. For reasonable systems, with CHAR_BIT in the 8-16 range, we could use a simple array or a bitset to store our values (though we'll need to convert the string's characters to unsigned char to avoid indexing out of range):

std::bitset<UCHAR_MAX+1> hist;
for (unsigned char c: a) {
    hist[c] = !hist[c];
}

When we have done this, we can simply use bitset::count() to tell us how many characters appear an odd number of times.

#include <cassert>
#include <iostream>
int main()
{
    assert(isPalinStr("aabbccd"));
    assert(!isPalinStr("aabbcd"));
}

A good practice to follow when bugs such as these are identified is to add tests which reproduce the bug; when they are made to pass, the tests remain as part of the program's test suite to guard against reintroducing similar bugs. That way, we gradually increase the number of valuable tests as we progress.

Our histogram maps char to int, which could potentially overflow (undefined behaviour again). We could use an unsigned type (which has well-defined overflow behaviour), but consider that we're only interested in whether the value is even or odd - so we could use a map from char to bool to store our values modulo-2.

Also, an unordered map is a pretty slow form of histogram. For reasonable systems, with CHAR_BIT in the 8-16 range, we could use a simple array or a bitset (though we'll need to convert the string's characters to unsigned char to avoid indexing out of range):

std::bitset<UCHAR_MAX+1> hist;
for (unsigned char c: a) {
    hist.flip(c);
}

When we have done this, we can simply use bitset::count() (or std::count() on an array) to tell us how many characters appear an odd number of times.

#include <cassert>
#include <iostream>
int main()
{
    assert(isPalinStr(""));
    assert(isPalinStr("a"));
    assert(isPalinStr("aa"));
    assert(isPalinStr("aabb"));
    assert(isPalinStr("abbcc"));
    assert(isPalinStr("aabccdd"));
    assert(isPalinStr("aabbccd"));

    assert(!isPalinStr("ab"));
    assert(!isPalinStr("abc"));
    assert(!isPalinStr("abbb"));
    assert(!isPalinStr("aabbcd"));
}

This code is repeated in both branches of the initial if:

That reduces the code to a much simpler version:

We have a strange test here:

    if (even % 2 == 0 && odd == 1) {
        return true;
    }
    else {
        return false;
    }

It doesn't matter how many matched pairs of characters we have, so the count of how many even histogram counts we found is irrelevant. This will fail "aba" for instance, as we only have a single even count. We really just need to test that we have 0 (for even-length strings) or 1 (for odd-length) odd-size counts.

Note also that the general pattern if (condition) return true; else return false; can always be replaced by return condition;.

This code is repeated in both branches of the initial if:

That reduces the code to a much simpler function:

#include <bitset>
#include <climits>
#include <string_view>

bool isPalinStr(const std::string_view s)
{
    std::bitset<UCHAR_MAX+1> hist;
    for (unsigned char c: s) {
        hist[c] = !hist[c];
    }

    return hist.count() <= 1;
}

#include <bitset>
#include <climits>
#include <string_view>

bool isPalinStr(const std::string_view s)
{
    std::bitset<UCHAR_MAX+1> hist;
    for (unsigned char c: s) {
        hist.flip(c);
    }

    return hist.count() <= 1;
}