Simple username validation

Question

This checks if a given username is valid, before doing any database lookups.

(I know it's possibly better to use regular expressions, but I wanted to be able to give specific errors.)

I'm curious if there is a cleaner way to represent the set of valid characters.

bool check_username_valid(const string& str, string &out_error) {
    static const struct LUT {
        bool a[256]{};
        LUT() {
            for (int x = 'a'; x <= 'Z'; x++) a[x] = true;
            for (int x = 'A'; x <= 'Z'; x++) a[x] = true;
            for (int x = '0'; x <= '9'; x++) a[x] = true;
            a['_'] = true;
        }
        inline bool operator [] (char x) const { return a[int(((unsigned char)(x)))]; }
    } allow;

    if (str.length() < MinUsernameLen) { out_error = "Username Too Short"; return false; }
    if (str.length() > MaxUsernameLen) { out_error = "Username Too Long"; return false; }
    for (char c : str)
        if (!allow[c]) {
            out_error = "Invalid Character in Username";
            return false;
        }
    return true;
}

I rolled back your last edit. After getting an answer you are not allowed to change your code anymore. This is to ensure that answers do not get invalidated and have to hit a moving target. If you have changed your code you can either post it as an answer (if it would constitute a code review) or ask a new question with your changed code (linking back to this one as reference). See the section What should I not do? on What should I do when someone answers my question? for more information — Sᴀᴍ Onᴇᴌᴀ
– Sᴀᴍ Onᴇᴌᴀ ♦, Commented Jan 13, 2021 at 17:57

Toby Speight · Accepted Answer · 2021-01-14 17:30:32Z

Instead of the loop, we can use std::all_of() (or std::any_of() with the opposite condition). That can make the code clearer, because it signals intent at the beginning, and we don't need to unpick control flow as we do with for/return. With C++20, we are able to pass a range object such as std::string as argument instead of an iterator pair (as we do in C++17).

And we have std::isalnum(), which is likely implemented as a lookup table like this (with the caveat that we need to be careful about signed char). It's more portable (the code we have will be wrong if there are non-alphabetic characters between A and Z, as there are on EBCDIC systems, for example) and easier to get right (witness the typo - Z instead of z - that means on ASCII-like systems we allow no lower-case letters).

We could change the interface to just return the error string, with a null pointer indicating success, to avoid the "out" parameter.

#include <algorithm>
#include <cctype>
#include <string>
    
// return a null pointer if valid, else a pointer to the error message
const char *check_username_valid(const std::string& str)
{
    if (str.length() < MinUsernameLen) { return "Username Too Short"; }
    if (str.length() > MaxUsernameLen) { return "Username Too Long"; }
    auto const char_permitted
        = [](unsigned char c){ return c == '_' || std::isalnum(c); };

    if (std::all_of(str.begin(), str.end(), char_permitted)) {
        return nullptr;
    }

    return "Invalid Character in Username";
}

If we'll be adding more permitted username characters, we might want to use the lookup-table approach - but we don't need a new type, and this static const can be built at compilation time:

#include <array>
#include <climits>

bool legal_username_char(unsigned char c)
{
    static auto const table
        = []{
              std::array<bool,UCHAR_MAX+1> a;
              for (unsigned char i = 0;  i++ < UCHAR_MAX; ) {
                  a[i] = std::isalnum(i);
              }
              // additional non-alnum chars allowed
              a['_'] = true;
              return a;
          }();
    return table[c];
}

That idiom is an immediately-invoked lambda, and it's very handy for creating complex constants like this. Note also the use of UCHAR_MAX rather than making assumptions about the size of the type.

Minor: no need for cast to int when indexing (a[int(((unsigned char)(x)))]). An unsigned char is perfectly fine as array index.

The problem with isalnum is that we may want to allow other characters which are allowed; e.g. a dot or hyphen or underscore, and then we end up with a long if statement — CaptainCodeman
– CaptainCodeman, Commented Jan 13, 2021 at 17:15
Yes - though we can certainly use std::isalnum() to populate the lookup table instead of the fragile tests we have now, so we could have the benefit of both. It might end up being extracted as a reusable function - I'll perhaps update to suggest that. — Toby Speight
– Toby Speight, Commented Jan 13, 2021 at 17:20
That's a good idea, using isalnum to initialize... But what's the benefit of using a fake loop like all_of instead of just a vanilla for loop? — CaptainCodeman
– CaptainCodeman, Commented Jan 13, 2021 at 17:23
How is std::all_of() "fake"? Its advantage is small here, but I think it does read slightly easier - the purpose is clear at the beginning of the expression, rather than only becoming clear once you're inside the loop. So a slight improvement in comprehension. It becomes clearer still in C++20 we we can pass the whole string as a range object. Plus, it's a reminder that the Standard Library provides lots of useful stuff! — Toby Speight
– Toby Speight, Commented Jan 13, 2021 at 17:30
Thank you for this, I really like using a lambda to initialize the static lookup table. Thanks also for the insight regarding non ASCII encoding. — CaptainCodeman
– CaptainCodeman, Commented Jan 15, 2021 at 13:00

Loki Astari · Accepted Answer · 2021-01-13 18:56:31Z

4

Everything Toby said.

The only extra thing I want to mention is using out parameters is a bit archaic. You can easily return multiple values from a function.

std::pair<bool, std::string> check_username_valid(std::string const& str)
{
    // STUFF
    return {false, "Bad Value"};
    // OR
    return {true, ""};
}

answered Jan 13, 2021 at 18:56

Loki Astari

97.7k5 gold badges126 silver badges341 bronze badges

\$\begingroup\$ That could be the cleaner way to avoid the out parameter. \$\endgroup\$

Toby Speight
– Toby Speight

2021-01-13 21:09:52 +00:00
Commented Jan 13, 2021 at 21:09
\$\begingroup\$ @TobySpeight Different, sure. More versatile, as the error-message may be dynamic? Also true. But cleaner? Debatable at best. \$\endgroup\$

Deduplicator
– Deduplicator

2021-01-13 21:19:07 +00:00
Commented Jan 13, 2021 at 21:19
\$\begingroup\$ Debatable, true; that's why I wrote "could be" rather than "is". :-) Definitely a technique to have in one's C++ toolbox, and upvoted for that alone. \$\endgroup\$

Toby Speight
– Toby Speight

2021-01-13 21:26:05 +00:00
Commented Jan 13, 2021 at 21:26

Add a comment |

Stack Exchange Network

Simple username validation

2 Answers 2

You must log in to answer this question.

Hot Network Questions

Simple username validation

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions