Twitch banned user names

Twitch source code was leaked into the internet, revealing emails and passwords, streamers salary, and also – a rare look inside a profitable piece of code.

A certain code snippet has found its way into r/badcode:
https://www.reddit.com/r/badcode/comments/q2mkue/so_umm_this_is_from_twitch_source_code_leak/

r/badcode - So umm this is from Twitch source code leak .

What entails it as a “bad code” exactly? Obviously seeing a long list of or statements is code smell, but is that all there is to it?

Before we start, it’s worth mentioning that some people suggested that the list gets generated by some automatic tool. However it’s not an excuse, it’s just shifting the blame to whoever wrote the tool.

Now let’s improve it!

It’s stored as a list in a txt file. This is bad. How can you manage that txt file of or statements? What do you do if you want to change it to something else? You go over and copy paste each phrase in a 1000 (or who knows how many more) lines?
For all purposes, a database for the banned words will do better. The automatic tool will insert new banned words into the DB, and whenever a user gets created, will check against the DB with a very similar syntax.

Is that all there is? A txt to database solution? Although it’s important as is, there can be even more improvement. Bare in mind not everyone will agree on that, but I believe true followers of S.O.L.I.D will.

This whole validation should not even be in SQL. Our DB handling modules should be clean and concise, so they are less prone to human errors and easier to maintain. For that purpose, some responsibilities are being put on a higher business layer. For example – checking if new user names aren’t already taken. It can be done (the wrong way) the same as in the code snippet. The function “Add()” will have SQL syntax that insert the new user name with conditional part – some SQL that checks if the name exists in the database. It’s ugly to imagine that. What’s more common is a flow such as this:

public ActionResult CreateUser(string userName, DBStore db)
{
    if(db.exists(userName))
    {
       // raise error or return information regarding the situation
    }
    db.insert(userName);
}

It’s clearer, makes everything fine grained, testable, and easier to maintain. The flow tells exactly what is happening and there is no reason to follow an enigmatic SQL syntax to understand what’s going on.

Similarly, for our original problem we should have a function or class that matches the username against regular expressions.

A few words of sympathy:
The programmers who made that probably know all of that, or at least that their implementation isn’t ideal They made that not because they are stupid or unprofessional, rather because they are employees working under pressure coming from their managers, bosses, and their deadlines. The temporary solution, became the permanent.

I bet you had some thoughts of your own while reading. Maybe you didn’t agree with me on some things or even not at all! After a discussion with a professional colleague of mine, I wrote another post describing the flaws I later found in my approach. Before checking it be sure to form an opinion of your own of what should be done and why – https://everlastingbits.com/2021/10/13/scrutinizing-different-design-decisions/

%d bloggers like this: