top | item 24019042

Reviewing the worst piece of code ever

143 points| micheleriva | 5 years ago |micheleriva.it | reply

207 comments

order
[+] bane|5 years ago|reply
Ha, this is nothing. We once fired a programmer and had her turn over her code after she failed to turn any work in after a few months. Her code was atrocious and also none of it worked...or even was runnable. The biggest sin was that she wrote all of her code in MS-Word, smart-quotes and all on all the literals. Never once ever tried to run or test any code, just thousands of lines of useless stuff that "looked" like code.

Oh yeah, she's also an adjunct CS professor at a local college.

[+] t0mmel|5 years ago|reply
I can’t help wonder “where are the colleagues?”

It’s not that I expect someone to try and salvage a situation where someone is coding in Word - but is there more to the story when you say this went on for months ? I don’t think I have ever been in an environment where that would ha e been allowed to continue for that long... ? Sounds like an environment scared of dealing with things ?

[+] nurettin|5 years ago|reply
We had a guy nicknamed jellyfish. He was responsible for crafting a frontend to the university's remote education system in flex and connect it to flash media server. After six months of "coding", he was asked for a demo and all he did was to showcase one of the existing products. After getting fired, he sued the university and got back in to get a few more paychecks before leaving. He was excentric, drawing role playing swords and printing entire books that he never read.
[+] colmvp|5 years ago|reply
> We once fired a programmer and had her turn over her code after she failed to turn any work in after a few months

How did that go un-noticed for months?

[+] gav|5 years ago|reply
I was on a project where a recently hired Senior Java Developer managed to avoid committing code for three weeks with various excuses. When we finally looked at their code, it was a couple of hundred lines code that looked like Java, but with no semicolons. It didn’t compile.
[+] skinkestek|5 years ago|reply
As someone who is well liked but often feel I'm not good enough:

Maybe I should bookmark this thread? This and the other examples in this thread is so crazy it should make me feel super productive any day ;-)

[+] Darmody|5 years ago|reply
We had a PHP programmer who didn't know the PHP tags (<?php ?>).

He would stare at the screen doing nothing and when every body else was busy and not paying attention to him, he would watch videos and visit gaming websites.

[+] centimeter|5 years ago|reply
Given that I've seen people like this IRL, I'm more curious how she got hired in the first place.
[+] jgwil2|5 years ago|reply
Do companies not have any kind of onboarding process that involves pairing and training? Even if someone knows how to code every project has its own tools and conventions that fresh hires cannot be expected to know. This is really on the company for just leaving her alone for months.
[+] colordrops|5 years ago|reply
How long ago was this? I've seen shit like this in the 90s, but I'd be surprised if businesses could tolerate waste like that right now.
[+] dr_kiszonka|5 years ago|reply
FWIW, Word's comments and changes tracking are pretty slick compared to, say, Sublime Text ;)
[+] naavis|5 years ago|reply
How do people like this get hired for a programming job in the first place?
[+] raverbashing|5 years ago|reply
> Oh yeah, she's also an adjunct CS professor at a local college.

Why is this not surprising at all.

Oh I know, because people think it's "ok" to put CS papers with pseudocode out there.

At this point I'm not sure you can even blame the person if the interview process was so bad that it let a person like that get in.

(Yes please interviewers bore me with another long winded description of FizzBizz or somethng because you don't know how to ask constructive questions that show competency)

[+] Izkata|5 years ago|reply
Code formatting is indeed minor here compared to the rest, but you missed the one that popped out at me: The opening curly braces are sometimes on the same line as the condition, sometimes on the following line.

That said,

> I’m absolutely sure that the code above is fake.

> That’s the first time that I see a synchronous SQL query:

  var accounts = apiService.sql(
    "SELECT * FROM users"
  );
Pretty sure you meant synchronous ajax/web request rather than sql query. This could be old code, because it totally used the be a thing: https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequ...

Edit: Actually hell, it still works, at least in Firefox. It just also logs the deprecation warning mentioned on that page. I thought this was already removed.

[+] Sniffnoy|5 years ago|reply
One bit of awfulness the article missed: Rather than scanning for the username and then checking whether the password is correct, it scans for a match on both username and password. So if you have the right username but the wrong password, it still has to check every single username to conclude this; it won't stop when it gets to the username you entered.

I mean, not that it should be scanning through a list to find the right username, but still...

(And yeah, I suppose that would allow for some sort of username enumeration via a timing attack, but somehow I don't think that's the reason they didn't do that here...)

[+] champtar|5 years ago|reply
You assume that username are unique in this DB, maybe they support multiple password :)
[+] CodesInChaos|5 years ago|reply
I don't think it matters. With an index on username you don't need to scan. Without an index (like in the example), you have to scan on average half the table for existing names and the full table for missing ones. Adding a check for the password won't affect performance in a way that matters.

The main reason why you can't do this in a correctly written application is that it's incompatible with salted hashes, since you need to get the salt from the database to verify the hash. (You should calculate the hash in the app server, since it's expensive and scaling app servers is usually easier and cheaper than scaling db servers)

[+] jlengrand|5 years ago|reply
A colleague of mine once turned in A FULL JAVA APPLICATION all written in the static void main method. Using booleans in databases polled at 25k/s to check for completion of async tasks. Worst is, it was working. I spent 6 months rewriting it, using it as a black blox to recreate the same system, including bugs because we were interfacing with other systems.

Infuriating given that we were a team people team, but also a great deal of fun from the engineering's side.

EDIT: Forgot to mention I was the junior dev and he had in excess of 15 years of experience :D

[+] kevsim|5 years ago|reply
A company I worked for acquired another company. When we finally got access to their code it was in two files: source.cpp and source2.cpp. The latter was introduced when they reached some limit of the visual c++ compiler at the time.
[+] ur-whale|5 years ago|reply
> he had in excess of 15 years of experience

From your description, it looks like the code was optimized to irritate while remaining functional.

Might he have been pissed with the institution he worked for?

[+] anamexis|5 years ago|reply
Hilarious, and a great breakdown of what's wrong with the code!

But also, in an article calling out programming falsehoods, this snippet is a bit odd:

> Even if apiService.sql returns a value synchronously (which I doubt), internally it have to open a connection to a database, make a query and send back the response, which (as you may have guessed) can’t be synchronous.

Sure it can, it's dead simple to write an API that accepts a database query and returns the result synchronously.

[+] ErikAugust|5 years ago|reply
But how would you call this API from the browser synchronously? I guess in theory there is a synchronous XHR call but I’ve never seen it used. That could be something you add to the problems.
[+] seanwilson|5 years ago|reply
I've seen something like this before that allowed login with any password as long as you gave a valid user name:

> if (hash(password == hashedPassword)) { login(username); }

"Don't write your own hash function" is common advice, but I would go much further and say don't write anything to do with passwords, logins, roles or sessions etc. if you can because one small mistake is all it takes to create a huge security hole.

[+] 8lall0|5 years ago|reply
This is genuinely stupid code from a first-week-junior dev who is still learning a lot.

The worst code i've ever seen was a PHP function that printed a menu (with relative permissions) from a database. Problem: it was O(n^3), 200 lines long, completely unmantainable and with random fails. Problem n° 2: it was written by our "best" programmer. It took me one hour to rewrite that into a 10 lines O(nlogn) function that my PM didn't want to use inside the framework because it was my first week.

[+] jarym|5 years ago|reply
But the employer wanted a full-stack developer, everything else is just details!

I’ve seen worse code than this. How about a hard coded backdoor check that looked like: || pw == “debug0”

[+] jl2718|5 years ago|reply
But the dev made 15 commits this week and knocked out 5 Trello tasks. All tests pass. Top performer.
[+] al2o3cr|5 years ago|reply
TBH if this is the "worst code you've ever seen", bless you sweet summer child.
[+] runawaybottle|5 years ago|reply
Right? I suppose he/she is focusing on the naive authentication piece, but I honestly thought most of the code was clean and sensible (the dev is just inexperienced). I’d rather teach this person the technology than someone who constructs labyrinth-like mental models, but knows the technology. They are code terror incarnate.

There are many people that could take that simple function, split it across three files, with various events being dispatched, with “elegant” switch cases, and over abstracted utility functions all over the place. These are the minds I fear, these monsters will eat your sanity with their contraptions.

You know nothing Jon Snow, this code is good. I know what the person was trying to do, and it can mostly be solved. Please purchase a ticket to a few React apps, or a home grown php framework. Then we’ll talk. You have not met a real mind eater yet.

[+] toomanybeersies|5 years ago|reply
> Here we can see the use of the double quotes for writing strings ... Here we can see single quotes ... This may not look important, but it’s actually telling us that the developer has probably copied some code from StackOverflow without even rewriting it following a common style guide for the whole codebase.

I have a habit of mixing double and single quotes, depending on what side of bed I woke up on. Doesn't mean I copied my code from Stackoverflow.

[+] jedimastert|5 years ago|reply
Does anyone have any experience with password-less login? Like were you just do the other half of a 2fa like an email? And I mean both experience as a consumer or developer.

I feel like if I were to make a service that required log in that's how I would do it, but with all the talking about it I've never actually seen it in the wild.

[+] uallo|5 years ago|reply
I use it in a side project. Reasons:

* B2B. The project is targeting companies where everyone relies on email.

* It is easier to implement. Compared to normal email/password-based authentication systems, it is almost exactly like the email verification step but it does not need any "forgot password" or "change password" functionality.

* Access tokens are temporary. If your DB ever gets hacked, the tokens are not valuable in the long run.

* Makes account reusing by multiple people more complicated. This is especially useful when your project uses seat-based pricing.

* Cheap "SSO". You can only use the service as long as you have access to the email address. This might be interesting for companies. People who leave automatically lose access (as soon as the token expires).

[+] mercer|5 years ago|reply
Do you mean stuff like Medium's 'magic link' approach?

enter email address, get a link via email, click on link: magically logged in!

If so, that's exactly what I've discussed with various clients to implement for their projects, because I agree that in many cases it's a really nice and, as far as I can tell, safe approach.

Personally I really don't like it, because I have a password manager and I rarely have my email open in my browser. But for many, if not most 'casual' computer users it does seem ideal to me.

[+] Macha|5 years ago|reply
It feels like a great way to lose users in the login process. They try to log in, go to their email, get distracted by a facebook/whatever login.

Even as a user, it's more friction than a social login or password manager. This is effectively what I needed to do when I used vatsim prior to using a password manager as they made you use a password set by the app that was 8 characters alphanumeric so I used to end up having to reset it a lot.

[+] msh|5 years ago|reply
As a user it's a pain. I might not want to sign in with my email on the phone or computer that I am trying to use some unrelated service on.
[+] flak48|5 years ago|reply
Firebase offers email login (passwordless) as an auth method. Where you are emailed a new URL that logs you in, each time you wish to.

It also offers SMS based login in a similar manner where no passwords are involved and you just copy a 6 digit or so number from a SMS you receive upon clicking 'Login'

In India this SMS based login method without passwords is the norm for almost all locally developed apps I've used (despite SMS as the sole authentication factor being not really secure due to the possibility of sim swapping, etc).

Disney Plus (Hotstar) in India has recently started mandating users to switch to such SMS based single factor auth (from password based login), presumably to add friction for account sharers.

[+] Kolians|5 years ago|reply
Microsoft uses password-less login across its services (like Outlook, Windows/MS Store, etc). I can still use my password if I absolutely want/have to, but once I enter my email, my Microsoft authenticator app on my phone prompts me for approval. Once I've given it, the site (or app or whatever) lets me in without having to enter the password. I really like it and sort of wish more sites did this.
[+] jackewiehose|5 years ago|reply
Discord is using user/password but every time I want to log in it says my IP-adress has changed and I have to use my email to verify my location. So it's basically the same as password-less email-login.

That's terribly annoying to me as a consumer so please don't do it.

[+] mrbonner|5 years ago|reply
Not the worst, though. It maybe insecure because the app was intended for internal use.

I did, however, see some shitty stuff in my job. Like, someone iterates a hashtable to find a key. I didn’t even know what to explain to that person when I reviewed the code.

[+] aljgz|5 years ago|reply
This is a piece of jewelry, compared to what I've seen.

Our team was called in to help on a strange case. An important organization had corruption in the management and they had contracted a mission-critical service, one that created income, to someone.

The code was written in asp (the project has started in 2016, 16 years after the last release). Database tables where named table_01 to table_36, columns named Column_01 to whatever. All values stored in database were "encrypted" using base64 most files where 10,000+ lines of repeated code, no modularity, no naming convention, not DRY.

All the income from the orders went to the programmer's personal bank account. The money would then be wired to the right companies.

Well, this was just the beginning, the code was much worse, I cannot bring any examples as I don't have the code and don't remember, but suffice to say that even though we had made our minds that we're going to handle this mess, but faced much much worse problem than we expected.

We came up with a smooth plan to transition to a good state.

Meanwhile, the corrupt management had been lobbying, got reinstated, made the version 2 contract with the same guy and the team I consulted was asked to just keep the version 1 running while the genius is coming up with more brilliant ways to write version 2 of pure shit.

[+] kanobo|5 years ago|reply
I'm sure most of us have seen far worse and more nonsensical code, it's a nicely designed snippet to teach basic security issues and show silly mistakes though.
[+] moltar|5 years ago|reply
I call bs on the code being real, because of triple equal operator. Doubt this developer would know such thing.
[+] jschwartzi|5 years ago|reply
Every JS IDE warns you about that so I could see a developer correcting the warning.
[+] Seb-C|5 years ago|reply
I have seen way worse than this.

In a "corporate [bull]shit" setting, I could easily imagine scenarios where the synchronous client-side query could make sense and be the best available option. Thankfully I have escaped this kind of workplace long ago.

The worse codebases IMHO are the ones that have random associative arrays that are exchanged and mutated randomly in any part of the code. Give it a few years of maintenance by bad developers, and you end up with a mess of conditional workarounds and variables that can contain up to a dozen of different data structures with inconsistent properties.

[+] rawfan|5 years ago|reply
I just saw an atrocious piece of Perl code that controls the flow of shipping containers for several large corporations. It’s completely unmaintainable and only the original author has the complete picture.

He was originally a trucker on office duty because of health issues. In the office he saw lots of inefficient thing so he bought a copy of „Perl for Dummies“ and started fixing.

The product itself is actually a huge success. No developer is willing to stay, though, so the guy just picks other people from the office and teaches them what he thinks is Perl.