Testing whether characters of a string are unique

Critiques (even pedantic ones) are welcome.

bool unique_chars(std::string &s) {
    if (s.length()>256) return false;
    std::bitset bs;
    for (auto &c:s) {
        if (bs.test(c)) return false;
        bs.set(c);
    }
    return true;
}

EDIT 1:

for (auto &c:s) {         //OLD
for (unsigned char c:s) { //NEW

Reasoning:

• char can be either signed or unsigned

• if signed then bs[] can be out of bounds

• casting to unsigned eliminates issue as negative values will wrap

EDIT 2:

This string is to be made up of characters from the standard ASCII character set, i.e. fitting into a 1 byte char.

EDIT 3:

Renamed function / added test harness with examples. Function parameter changed to const, as string not being modified.

#include 
#include 

bool areCharsUnique(const std::string &s) {
    if (s.length()>256) return false;
    std::bitset bs;
    for (unsigned char c:s) {
        if (bs.test(c)) return false;
        bs.set(c);
    }
    return true;
}

int main() {
    std::string s1 = "hi there";
    std::string s2 = "hey man";
    std::cout << areCharsUnique(s1) << std::endl; //0
    std::cout << areCharsUnique(s2) << std::endl; //1
   return 0;   
}

@Wayne Conrad has made some good points. I just have a few additional ones:

-
I'm not sure about checking against a maximum value, but I'll address it anyway.

In case you'd like to change the default 256, consider having a template parameter. That way, the value can be changed at just one location. It'll also give more meaning to that value.

template 
bool unique_chars(std::string &s) {
    if (s.length() > maxChars) return false;
    std::bitset bs;

    // ...
}

-
To slightly add on to Wayne's point about const, make sure the argument is const& so that no unnecessary copying is done. In addition, declaring the string argument as const will allow it to be passed in while avoiding casting.

-
Consider renaming the function to something like areUniqueChars(). Otherwise, it may sound like it's returning the number of unique chars.

-
In C++, prefer to put unary operators (such as & and *) next to the type:

std::string& s;

for (auto& c : s) {}

StackExchange Code Review Q#39579, answer score: 10