6 ms·
What we learn from twitch source code leak
- thdc 5y agoAt least for me, I never expected production code to be this bad until I spent a few years working professionally. One company I worked at had poor code quality, but I chalked it up to poor engineering culture - a lot of outsourced crap code and no time dedicated to go back to clean it up. Another company also had a code base with terrible quality but I figured it was because it was a startup and they were rushing to get features out. None of the companies I've worked for are big ones, and I've always wondered if these levels of quality were the norm, and leaks like this really give me insight to what would otherwise be private code. Of course, Twitch was also a startup so maybe this was a hack that made it into production as the author notes, although I don't imagine a better solution would've taken much more effort to implement. Well you could also argue that I'm the common denominator here and the code isn't poor per se, it's just that my expectations were too high when coming into the professional scene - which is what I'm leaning towards currently - I don't really know what to think.
- everlastingbits 5y agoI'm also super interested about how code looks like in elite organization! In my previous work place I felt exactly the same as you.. Lucky for me there are many seniors with years of experience under their belts where I'm currently working.
- mftb 5y agoI think finding mentors is a great idea, but I think one thing you, and the parent comment are missing is, an "elite organization" in what arena? Of what sort? For a truly different software development process, with totally different requirements, checkout stuff like this - https://www.fastcompany.com/28121/they-write-right-stuff https://www.fastcompany.com/28121/they-write-right-stuff. There are other similar write-ups from different kinds of organizations out there as well.
- TedDoesntTalk 5y ago> https://www.fastcompany.com/28121/they-write-right-stuff https://www.fastcompany.com/28121/they-write-right-stuff This is an amazing read on the software controlling the space shuttle from 1996. Thanks for sharing!
- mftb 5y agoYour welcome. I read it back when, and it's been helpful to me over the years.
- thdc 5y agoNice article. Thorough documentation (history of code database mentioned in article) is definitely one of those things I'd like as I often find myself wondering why code is the way it is, trying to look back through to the original change that introduced it, and finding nothing - also everyone involved with that change is long gone from the company. Also thorough specifications are pretty unheard of so that's also great. And devs that actually test their code thoroughly. For me elite just might be a company that meets my standards, which I realize is a useless description for anyone that's not me now. In fact, it's a lot of personal opinion - mainly code that's easy to modify and reason about I guess. I realized I said "this bad" in my original post but, to be clear, I don't have any serious issues with the Twitch snippet - I meant "this" as software in general. The only nit I have is using `like any array(['%p1%', '%p2%'...])` seems like a more elegant and straightforward(?) way to write the logic.
- pkrumins 5y agoThis is exactly how code looks everywhere. It does the job. It's the greatest code ever written because it's in production. The perfect code you never wrote is in your dreams. Services don't run on dream code. They run on code in production.
- jandrewrogers 5y agoBeautifully crafted production code exists but the necessary conditions are almost never consistently available -- it requires a lot of time and energy to fight code entropy. Even most people that write software don't care that much; like the business, anything that meets the acceptance criteria is usually considered "good enough". And most professional, production code is the accumulated cruft of that reality. In the rare cases where you do see elegant, clean, high-quality production code, it is usually because 1) only a very small number of people are writing that code, 2) there is a strong cultural norm of rigorous code hygiene among those people, 3) they do not have prohibitive time pressure to ship, and 4) the requirement to maintain backward compatibility across versions of the code is weak. In practice, these conditions are very rare. I've mostly seen it in what is essentially hobby code, where the craft was a large part of the objective (and hobby code that becomes production code tends to quickly take on the characteristics of other production code). There simply aren't enough people that care about code quality for its own sake, and the economics of maintaining very high-quality code rarely makes sense in practice.
- thinkingkong 5y agoWhat we can really learn (or remind ourselves) is that code “quality” is rarely representative of company value.
- Sebb767 5y agoPeople pay you for your product, not what's running under the hood. As long as you can sustain feature development and/or enough pull, code quality is absolutely irrelevant. Also, any large codebase will develop some wharts due to circumstances you can't see when simply reading the code.
- bserge 5y agoHuh, maybe the thinking that everything has to be perfect is rooted in products where people do pay for what's under the hood? I.e. literally cars.
- TeMPOraL 5y agoCars are just a ruthlessly optimized collection of parts that, individually, few people understand. Not unlike software. Unless you're a specialist, you wouldn't know "bad insides" of a car from "good insides" if you took a look under your car's hood. The way I see it, programmer perfectionism comes from learning - the projects you do for yourself, early on, are small enough that you can hit perfect trade-offs on your limited needs, and you aren't under time pressure. You quickly learn you can achieve near-perfection - a lesson which stops applying when working with any nontrivial codebase at work.
- burnt_toast 5y agoThere's plenty of cars out there that mechanics think are designed like junk yet drivers (users) still buy / use them. (Not sure where I was going with this)
- bserge 5y agoEven then, buyers care about performance.
- i_like_apis 5y agoSometimes what works is the right choice. Duct tape doesn't look nice but it gets the job done and fast.
- deleted 5y ago[deleted]
- platz 5y ago> Is that all there is to it? A txt to database solution? > This whole check should not even be in SQL. > Similarly, for our original problem we should have a function or class which purpose is to check whether a word matches against many regex phrases. Interesting conclusion. A Very confusingly-worded article.
- everlastingbits 5y agoThanks for pointing that out! English not my first language.. I changed all of the above into something better, I hope. :D
- trjordan 5y agoA database is the wrong solution to this problem. The problem is spam detection and blocking. This solution is simple and good. A developer on the receiving end of a "add an item to this list" bug report has a clear, well-trodden way to do it: update the text file. They have a modern editor that handles big files just fine. They frequently deploy new code to production, including testing it. Moving to a DB means that it needs an interface in production, because presumably Twitch doesn't let their devs run random commands on production boxes. It means ACLs and a sync between dev and prod. It means another moving piece when Twitch spins up a disaster recovery test. The full-powered, industry-standard solution is an AI-based spam detector with some sort of lag on blocking (e.g. shadow banning). This requires inputs to train on, such as reports from Twitch moderates, which might need oversight. It requires ML engineers and quality heuristics. This stuff is getting cheaper, but it's a hell of a lot more expensive than an ugly text file.
- outime 5y agoThis snippet has been constantly criticized and somehow it makes me think that the people who loudly complain about it have never worked in any kind of big company - which is fine but it shows a lack of empathy, experience or both. You can go to any big company around the world and you'll find duct tape because you know what, if it works, it isn't terrible to use/update and doesn't leak millions in lost revenue then there's no valid reason to spend dev time on that. This most likely was a temporary solution that grew over time and was left like it, they just kept adding new combinations. It works. If you want to add a new combination you just add it and run a migration (or similar, I've not worked on Twitch but I assume it's not the most difficult thing in the world if it's still like this). I understand that this can feel like an itch that you really need to scratch (I also feel it) but if you literally see the source codes of any big company you'll find stuff that anyone can criticize from their chair with little effort. Let's now go build a tremendously successful platform like Twitch without cutting a single corner ever and see how that goes.
- jalino23 5y agothis is true, the same people who complained about this are prob the one with least experience
- vinay427 5y agoTo be fair, at least as of this writing, TFA does end with this which seems to align with your view: > 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. What was suggested as a “temporary” solution, turned into the permanent.
- PragmaticPulp 5y ago> because you know what, if it works, it isn't terrible to use/update and doesn't leak millions in lost revenue then there's no valid reason to spend dev time on that. Really bad code grinds progress to a halt when developers have to spend all of their time fixing tech debt or working around fragile codebases to get anything done. However, I don't see anything in this code that would match that description. In fact, anyone can take one look at this and know exactly what it does and exactly how to modify it. Counterintuitively, perfectly good code also grinds progress to a halt if developers become too focused on doing things the "right" way instead of shipping reasonable code that works. Developers who get lost pursuing a platonic ideal of the perfect code will be perpetually disappointed inside of real-world constraints. Perfect is the enemy of good. It's definitely not fair to criticize a single snippet extracted from an entire company's code base. It's turning into a cheap way to dunk on a company from the sidelines while ignoring that the company has done a good job of scaling a video delivery platform and community to a massive number of users.
- rad_gruchalski 5y agoNOTHING. There, I answered it for you. Wow, people have problems. Does it work? It does. How often is this executed? On sign up and maybe nick change. Is it encapsulated? Yes, it is. Move along, nothing to see here.
- politelemon 5y ago> It’s stored as a list in a txt file. It is not. You've misunderstood that code snippet.
- 29athrowaway 5y agoWhat you should learn about that leak is that: - Bad code lives in the dark. Code that is visible by many others never looks like this. - Shame is good to some extent. It keeps people accountable. - When closing tickets is more important than actually making real improvements, code looks like this.
- shadowgovt 5y ago> Bad code lives in the dark. Code that is visible by many others never looks like this. As someone who has attempted to make a modification to OpenSSL, I feel like this claim needs a big old "[citation needed]" banner.
- 29athrowaway 5y agoOpenSSL code does not look like the code cited in the article. OpenSSL is known to be complex, hard to audit, etc. But they do have standards. You may want to take a look at LibreSSL or BoringSSL instead if you are looking for something more modern and maintainable.
- shadowgovt 5y agoI mean, it hasn't grown to a thousand entries, but https://github.com/openssl/openssl/blob/master/crypto/http/http_lib.c#L96 https://github.com/openssl/openssl/blob/master/crypto/http/h... There's also the parameter parser builder at https://github.com/openssl/openssl/blob/master/crypto/param_build.c#L160 https://github.com/openssl/openssl/blob/master/crypto/param_... This kind of pattern of repeated application instead of iterating over a data structure ends up fairly common in any code base I've ever seen, public or private. The machinery to build a list loader and iterator always just seems like more code than one wants to write when one is tasked with adding one more element to the existing sequence of homogeneous operations varying the data.
- IshKebab 5y agoUhm wasn't this code part of a script to gather training data for AI? If so it would be run offline and not as part of the actual site. I think it's perfectly reasonable for a one-off script to be a bit hacky.
- planb 5y agoInstead of arguing if this implementation is good or bad, can we please talk about the elephant in the room? What exactly is this snippet supposed to do? Filter out „bad“ usernames by exact match? How did they even come up with this list? There‘s infinite possibilities to spell profanity or insulting phrases, so aren’t they fighting windmills here?
- shadowgovt 5y agoYes. It's the sort of unwinnable game you play when you are a big company and want advertising and a reputation as a "safe(ish)" space. What is considered offensive constantly changes, and you constantly update a list like this to keep up. Facebook has a list like this. So does Twitter. Google has several and an ongoing initiative to consolidate them behind a shared service.
- pkrumins 5y agoThis is the best code ever written. It does the job and it's in production. The perfect code you never wrote is in your dreams. Services don't run on dream code. They run on code in production.
- yablak 5y agoI like this code: - it's got a history/blame with associated bugs linked in the changes, including all the reasoning behind every line. - it can be versioned into rollouts and rolled back if it interacts with another system in a bad way. - it's dead simple and self describing. What's not to like?
- phillipseamore 5y agoThis looks to me like a single-use check of the user database for "bad" usernames. A manager comes in, says he's gotten complaints about "bad" usernames and asks an engineer to check the DB and give him a list of them. This does _not_ look like something used in production.
- kcartlidge 5y ago- How the Code Probably Got There The login names are very specific, despite the use of 'like' and wildcards. It looks like an exception list that is added to as login names are chosen, spotted, and banned. Which means someone just pops into the code and adds a new entry as a 'bad' login comes to light. It isn't expected that it will proactively block bad ones but that it will stop already-used bad ones from recurring. Which explains why it looks as it does; it has just grown from a quick hack way back when. - About the Code Quality I'd guess none of us would set out to write it like this if we knew how it would grow, but for those original handful of exclusions a database would have been overkill. There comes a point where you start considering refactoring, but in a business with priorities it isn't unusual to look at code like this and say it's encapsulated, efficient, version-controlled, and in just one place, so other things are more important than changing what isn't actually broken (meaning the location of the code/data; I'd probably have issues with the actual logic but that's a different thing). - And the Straw Men As for the article this thread is discussing I'm not a big fan. The second half says "This whole validation should not even be in SQL". Yet the only reason that's even up for discussion is because the first half suggests it. The code certainly didn't. Which makes most of the article pointless. And the reasons to complain include "how can you manage that txt file of or statements?", "What do you do if you want to change it to something else?", and "copy paste each phrase in a 1000 lines?". It's simple - you manage the txt file (actually a file of source code, but whatever) by editing it and versioning it. Want to change it? Then change it. There is no reason whatsoever that it would result in copy/pasting 1,000s of lines when only the ones changing need changing. - And the Overhead of the Proposed Change A tool? To maintain a database list? Why waste sprint time creating a tool to replicate basic textual operations in code that is simple, isolated, and fast. - Summary Sorry for the rant. There's good code and bad code, but there's also good enough code and whilst as I said I'd never set out to write this there's no real reason to change it. YMMV. Five developers have five opinions plus a sixth for the consensus. We won't all agree. I can live with that.