Details
Joined devRant on 9/12/2016
Join devRant
Do all the things like
++ or -- rants, post your own rants, comment on others' rants and build your customized dev avatar
Sign Up
Pipeless API
From the creators of devRant, Pipeless lets you power real-time personalized recommendations and activity feeds using a simple API
Learn More
-
This code review gave me eye cancer.
So, first of all, let me apologize to anyone impacted by eye cancer, if that really is a thing... because that sounds absolutely horrible. But, believe me, this code was absolutely horrible, too.
I was asked to code review another team's script. I don't like reviewing code from other teams, as I'm pretty "intense" and a nit-picker -- my own team knows and expects this, but I tend to really piss off other people who don't expect my level of input on "what I really think" about their code...
So, I get this script to review. It's over 200 lines of bash (so right away, it's fair game for a boilerplate "this should be re-written in python" or similar reply)... but I dive in to see what they sent.
My eyes.
My eyes.
MY EYES.
So, I certainly cannot violate IP rules and post any of the actual code here (be thankful - be very thankful), but let me just say, I think it may be the worst code I've ever seen. And I've been coding and code-reviewing for upwards of 30 years now. And I've seen a LOT of bad code...
I imagine the author of this script was a rebellious teenager who found the google shell scripting style guide and screamed "YOU'RE NOT MY REAL DAD!" at it and then set out to flagrantly violate every single rule and suggestion in the most dramatic ways possible.
Then they found every other style guide they could, and violated all THOSE rules, too. Just because they were there.
Within the same script... within the SAME CODE BLOCK... 2-space indentation... 4-space indentation... 8-space indentation... TAB indentation... and (just to be complete) NO indentation (entire blocks of code within another function of conditional block, all left-justified, no indentation at all).
lowercase variable/function names, UPPERCASE names, underscore_separated_names, CamelCase names, and every permutation of those as well.
Comments? Not a single one to be found, aside from a 4-line stanza at the top, containing a brief description of that the script did and (to their shame), the name of the author. There were, however, ENTIRE BLOCKS of code commented out.
[ In the examples below, I've replaced indentation spacing with '-', as I couldn't get devrant to format the indentation in a way to suitably share my pain otherwise... ]
Within just a few lines of one another, functions defined as...
function somefunction {
----stuff
}
Another_Function() {
------------stuff
}
There were conditionals blocks in various forms, indentation be damned...
if [ ... ]; then
--stuff
fi
if [ ... ]
--then
----some_stuff
fi
if [ ... ]
then
----something
something_else
--another_thing
fi
And brilliantly un-reachable code blocks, like:
if [ -z "$SOME_VAR" ]; then
--SOME_VAR="blah"
fi
if [ -z "$SOME_VAR" ]
----then
----SOME_VAR="foo"
fi
if [ -z "$SOME_VAR" ]
--then
--echo "SOME_VAR must be set"
fi
Do you remember the classic "demo" programs people used to distribute (like back in the 90s) -- where the program had no real purpose other than to demonstrate various graphics, just for the sake of demonstrating graphics techniques? Or some of those really bad photo slideshows, were the person making the slideshow used EVERY transition possible (slide, wipe, cross-fade, shapes, spins, on and on)? All just for the sake of "showing off" what they could do with the software? I honestly felt like I was looking at some kind of perverse shell-script demo, where the author was trying to use every possible style or obscure syntax possible, just to do it.
But this was PRODUCTION CODE.
There was absolutely no consistency, even within 1-2 adjacent lines. There is no way to maintain this. It's nearly impossible even understand what it's trying to do. It was just pure insanity. Lines and lines of insanity.
I picture the author of this code as some sort of hybrid hipster-artist-goth-mental-patient, chain-smoking clove cigarettes in their office, flinging their own poo at their monitor, frothing at the mouth and screaming "I CODE MY TRUTH! THIS CODE IS MY ART! IT WILL NOT CONFORM TO YOUR WORLDLY STANDARDS!"
I gave up after the first 100 lines.
Gave up.
I washed my eyes out with bleach.
Then I contacted my HR hotline to see if our medical insurance covers eye cancer.32 -
Not by me, but by my friend
He write a shell command to alias 'cd' into 'rm -rf' and then print out 'hehe', then save the command to bash_profile
Me? I put that command to our engineer's slack channel and wait for a natural selection does its job2 -
string excuses[]={
"it's not a bug it's a feature",
"it worked on my machine",
"i tested it and it worked",
"its production ready",
"your browser must be caching the old content",
"that error means it was successful",
"the client fucked it up",
"the systems crashed and the code got lost" ,
"this code wont go into the final version",
"It's a compiler issue",
"it's only a minor issue",
"this will take two weeks max",
"my code is flawless must be someone else's mistake",
"it worked a minute ago",
"that was not in the original specification",
"i will fix this",
"I was told to stop working on that when something important came up",
"You must have the wrong version",
"that's way beyond my pay grade",
"that's just an unlucky coincidence",
"i saw the new guy screw around with the systems",
"our servers must've been hacked",
"i wasn't given enough time",
"its the designers fault",
"it probably won't happen again",
"your expectations were unrealistic",
"everything's great on my end",
"that's not my code",
"it's a hardware problem",
"it's a firewall issue",
"it's a character encoding issue",
"a third party API isn't responding",
"that was only supposed to be a placeholder",
"The third party documentation is wrong",
"that was just a temporary fix.",
"We outsourced that months ago.","
"that value is only wrong half of the time.",
"the person responsible for that does not work here anymore",
"That was literally a one in a million error",
"our servers couldn't handle the traffic the app was receiving",
"your machines processors must be too slow",
"your pc is too outdated",
"that is a known issue with the programming language",
"it would take too much time and resources to rebuild from scratch",
"this is historically grown",
"users will hardly notice that",
"i will fix it" };11 -
Just for fun I am making an RPG Maker game called "IT Quest" where you go on a 40 hour long quest just to get the security team to modify a user. The game will feature tons of mandatory side quests and a convoluted plot that requires descending into the depths of the server room to find a virgin followed by a sacrificial ritual over the broken fax machine. And ultimately the security team just closes your request without telling you why and you have to fight the final boss of the game, Zeromus who runs security. When you defeat him you get the golden CAT5 cable of time which you beat the person who closed your request with until he reopens it and does his damned job.12