Details
-
LocationLondon, United Kingdom
-
Github
Joined devRant on 11/20/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 -
So I need to create a nice new web app. Let's look at some cool JS frameworks that I can work with.
*5 mins later* Hm, Angular sounds good, is there any good competitor?
*5 mins later* Wow, React sounds awesome as well. Let me learn it.
Google search result:
"Planning to use react? Check out Vue JS first"
*5 mins later* Ok so vue seems faster than React and much easier to learn. Let me see if Vue is the final choice.
Google search result:
"Angular VS Knockout VS Ember VS React VS Mithril VS Mercury VS Ractive VS Vue VS Riot"
Nope, fuck it63 -
I played a lot of Command & Conquer when I was younger, and I remember going through the files for C&C: Red Alert. I found one that had all the units names and properties, and wondered what happened if I changed a value. So I changed grenadiers attack speed to something ridiculously fast, and found that it actually changed it in the game!
The light bulb went off in my head, and I then created new units:
- Albert Einstein that shot electricity
- Attack dogs that launched missiles
Granted the animations didn't exist for these so it defaulted to playing their death animations when attacking, which was amusing.
That was the ah-ha moment for me that lead me to pursue programming. It was just so much fun!4 -
After moving to SSD I noticed I'm too slow for my computer. When it finishes working I can feel it asking me "you done thinking m8?"4
-
I put an Easter egg into a product, that if you enter the string "final countdown" into the stock code search field, it plays a YouTube vid of Europe's "The Final Countdown", in a hidden div. It's an in-joke for a few people in the company.
A well meaning maintainer with no sense of humour or judgement takes over and goes on the warpath against any hardcoded strings. The secret code gets moved into a config file.
A third developer changes the deployment script so that it clears any configs that aren't explicitly set in the deployment settings.
So the secret code is now "".
Literally every PC in the stock buying department is now blaring out "The Final Countdown" at top volume.
...Except none of them have speakers, so it remains this way for over a year and two more changes of maintainer.
I just noticed this afternoon and quietly re-hardcoded the string. The buying dept.'s PCs will silently sing no more.31 -
Me: 1 is something, 0 is nothing, NULL is the absence of things
JuniorDev: wut
Me: You've got pizza in a box, that's 1. If there's no pizza in the box, that's 0. If there's no pizza and no box, that's NULL.
JuniorDev: OOH so there's no object to reference if I ask for a slice!
Me: *small tear*
Always explain things in terms of pizza. Always.25 -
!rant
System.out.println(age);
if (birthday.isToday())
age++;
System.out.println(age);
Output:
17
1811 -
So you build a beautiful site; you spend good time on UX, refactoring, server optimisation, getting good page load speeds, SQL all optimised - life is good.
Commercial team comes in and slaps clickbait, generic advertising, tracking scrips over the lot.
Page loads go from a second to 30 seconds and even though you made sure all those crappy ad scripts are asynchronous pages still hang most times. PingdomTools lists your page scripts as going from 40 files to over 900... now users are ringing me up giving me grief about how slow this new company website is...5 -
Sometimes, you'll be tempted to copy-paste some piece of code from somewhere (stack overflow, w3schools, etc). But instead, read the code there and re-type it. Will help you to understand it better.2