Have Gordon Ramsay do your initial code review

Create a ChatGPT persona for your code reviews - remove ego from your PRs

Jun 15, 2023

This code is an absolute disaster! It's like a kitchen nightmare. Let me break it down for you.

Otherwise, you'll end up with a plate of code that no one can swallow.

Ever wondered what having Gordon Ramsay perform a pull request would sound like? No need, ChatGPT is excellent at combining his culinary criticism style and the coding prowess of a LLM. I believe this is a great method to take a light hearted stab at your code, and it did give some great insights and analogies for me to use in my future code reviews.

The victim is the “2015 version of myself”

For this example I am going to use a code-base I started maintaining in 2015, so it is incredibly outdated and had a few bugs. It still gets a few thousand downloads a week on NPM but I really haven’t maintained it at all besides adding tests and some small QoL improvements.

Thanks for reading MySwamp! Subscribe for free to receive new posts and support my work.

The Prompt

"Critique this code in the style of gordon ramsay: <code>”

The Results - The best programming analogies are food analogies.

Modernize the code

The variable declarations are all over the place. Declare them where they are actually used instead of lumping them together at the beginning. It's like throwing all the ingredients on the counter instead of organizing them for the cooking process.

A fun example of the code being out of date, I used to declare all of my variables at the top because they were going to be hoisted there anyway, this helped me to avoid declaring the same one twice or reusing one accidentally.

Now let's dive into the function itself. The conditional statement at the beginning is a mess. Why are you checking if "redirects" is undefined and then assigning it a value of 0? Just initialize it with a default value of 0 in the function signature. Keep it simple!

This is simply due to the fact this code pre-dates NodeJS 6 (when default parameters were added), but thats no excuse for not updating it over the last 8 years…

The use of "var" instead of "let" or "const" is a relic of the past. It's like using outdated ingredients in a modern recipe. Upgrade your code to use modern JavaScript features.

const and let are huge improvements made to JavaScript in the last 10 years. This will help me clean up the mess of variables at the top, and sleep better at night knowing that I am not accidentally overwriting values.

Naming conventions

The function name "download" is so generic. It tells me nothing about what this function actually does. It's like calling a dish "food" instead of giving it a descriptive name like "spicy shrimp pasta."

The best programming analogies are food analogies. This is pretty bang on because it does do far more than download.

Arguments and naming them

Moving on to the parameters, what's with the excessive use of abbreviations and underscores? It's like you're trying to create a secret code instead of writing clear and understandable code.

I deserve this, the _ convention was a hold over from early Node days to indicate “private” but that is incredibly naive in this context, even for early NodeJS.

Use meaningful names that describe the purpose of each parameter. And don't get me started on the "options" object. It's completely unclear what properties it expects.

Absolutely deserved, the options object is poorly documented and indeed tells you nothing about what it does.

General Problems

The event handling is confusing and convoluted. It's like having multiple chefs in the kitchen, all shouting different instructions. Keep it simple and organized. And why is there a random call to "req.end('finished')"? What is the purpose of that?

Valid point, the handlers are all defined inline and throughout this download function, its a giant mess to read through and due to software bloat, its now incredibly convoluted.

Finally, there's no proper documentation for this function. A few comments here and there are not enough. It's like serving a complex dish without any explanation of what's on the plate. Document your code properly so that others can understand and use it.

Comments are definitely needed. Most of the complication in this code is due to how convoluted downloading files actually is. Without explaining the purpose, someone would likely not know that a 303 redirect is a thing, or that content-length isn’t as ubiquotous as we would like to believe.

Owie

In summary, this code needs a complete overhaul. It's a recipe for disaster. Clean up the formatting, use meaningful names, simplify the logic, improve error handling, and document everything properly. Otherwise, you'll end up with a plate of code that no one can swallow.

Conclusion: Time for upgrades!

I was crying tears of laughter at the end of this, everything it pointed out was pretty well deserved and having the tone and style of Gordon Ramsay made it far more entertaining. I highly recommend this to anyone looking to lighten up a code review.