Testing whether characters of a string are unique Critiques (even pedantic ones) are welcome.

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


EDIT 1:

for (auto &amp;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 &lt;iostream&gt;
#include &lt;bitset&gt;

bool areCharsUnique(const std::string &amp;s) {
    if (s.length()&gt;256) return false;
    std::bitset&lt;256&gt; 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 &lt;&lt; areCharsUnique(s1) &lt;&lt; std::endl; //0
    std::cout &lt;&lt; areCharsUnique(s2) &lt;&lt; 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 &lt;std::size_t maxChars = 256&gt;
bool unique_chars(std::string &amp;s) {
    if (s.length() &gt; maxChars) return false;
    std::bitset&lt;maxChars&gt; bs;

    // ...
}

To slightly add on to Wayne's point about const, make sure the argument is const&amp; 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 &amp; and *) next to the type:

std::string&amp; s;




for (auto&amp; c : s) {}