Writing Better Code: The Quick and Easy Guide to Doing the Title of this Blog Post

Given this snippet of PHP code, how much could you improve it?

// INPUT: an array
// OUTPUT: message to the screen
// UPDATED: 2017-6-5
// AUTHOR: Elongo Wendrico
function displayedDetaileMessages($passed_cnt)
{
    $return = false;
    if(length($passed) == 0){
        $return = true;
    }else{
        $return = false;
    }
    return $return;
}

Short answer to “how much could you improve it?” is “quite a bit”.
So, the first order of business is the code itself. Do you see that trash inside of that function? It’s comically saturated with nonsense, and we’re going to get all of that nonsense out and make this code tight as a drum. We’re going to make this PHP so good that it will become the ultimate oxymoron. (I’m kidding. I don’t hate PHP, I just like every other language more)
We could keep functionality the same if we removed that silly ‘else’ since $return is already set to the exact same value as it would be inside of the ‘else’ statement.

$return = false;
if(length($passed_cnt) == 0){
    $return = true;
}
return $return;

Next, why are we creating a variable and immediately returning it? That’s silly and wasteful, so let’s get that out of there.

if(length($passed_cnt) == 0){
    return true;
}
return false;

Additionally, why are we even using an ‘if’ statement at all? If we are returning true or false, just return the expression itself. Damn. This is a worst-case scenario, but it is actually something that I encountered recently, so be aware that silliness like this could be occurring at any time.

return (length($passed_cnt) == 0);

Now, all you expert nerds are probably readjusting your glasses, or cracking your fingers and preparing to type out a flurry of scathing comments on your vintage buckling spring IBM keyboard with a custom Arduino converter that lets you plug it into your USB port (which I am envious of). Well just settle down for a second, because I haven’t forgotten that there is still some nonsense here. We want to use triple equals!
…kidding. Triple equals a good habit to get into, but it’s not what we are after here. What we want to do is avoid the function call and comparison, and just run the isEmpty() function on this.

return isEmpty($passed_cnt);

OK, that feels nice, doesn’t it? Now we have trimmed down the junk code and have something that IS STILL ABSOLUTELY HORRIBLE.

// INPUT: an array
// OUTPUT: message to the screen
// UPDATED: 2017-6-5
// AUTHOR: Elongo Wendrico
function displayedDetaileMessages($passed_cnt)
{
    return isEmpty($passed_cnt);
}

Look at all of this. There are still a lot of things here that need to be improved in order to make this good code, and they are not readily apparent to a surprisingly large number of folks.

Things like:

  • Long variable and function names
  • Poor choice of variable and function names
  • Spelling errors
  • Abbreviations
  • Comments

Choosing the right names will make your code easier to comprehend by not only your greasy self but by any other programmer who has the misfortune of dealing with the code.

Let’s tear this bullshit apart like Ongo Gablogian the art collector.

// INPUT: an array
// OUTPUT: message to the screen
// UPDATED: 2017-6-5
// AUTHOR: Elongo Wendrico
function displayedDetaileMessages($passed_cnt)
{
    return isEmpty($passed_cnt);
}

 

if(isPHPFrom2004()){echo

Comments about input and output shouldn’t be there, doubly-so since this isn’t a strongly-typed language. If this is a strongly-typed language, then…I don’t know. I don’t know your rules. Your code should indicate or give clues as to what is being passed in and out. In this case, $passed_cnt is being passed, but it is probably intended to be an array, which leads us to wonder what $passed_cnt is actually supposed to be.
I see “cnt” and I think of two things, one of them is “count”, and those are usually a single value, not an array.
Let’s change $passed_cnt to something else for this.

 

Also, you may notice that this example doesn’t even take an array as input or output a message to the screen. There is a lesson here: Keeping your code DRY doesn’t just involve your code, it involves comments. If your code changes, you would need to update your comments as well, otherwise, you are giving the next poor soul conflicting information.

// UPDATED: 2017-6-5
// AUTHOR: Elongo Wendrico
function displayedDetaileMessages($detaileMessages)
{
    return isEmpty($detaileMessages);
}

Next, take a look at that “updated” comment; this is just another point that needs to be changed when editing the code. Keep it DRY, and check the Git logs or whatever. Keeping it is dumb so delete it.

// AUTHOR: Elongo Wendrico
function displayedDetaileMessages($detaileMessages)
{
    return isEmpty($detaileMessages);
}

Are you getting irritated by that glaring spelling error? Spelling errors and abbreviations means that you need to guess what the correct variable name is, and makes searching more difficult. Use correst speeling in ur code plz and no abreviatns especially for words like title and count.

// AUTHOR: Elongo Wendrico
function displayedDetailMessages($detailMessages)
{
    return isEmpty($detailMessages);
}

Is that function name descriptive enough? Is it too long?
The “displayedDetailMessages” sounds like it returns the messages that are displayed, but this code just checks if the passed variable (probably an array) is empty. Let’s fix this. I’m using “is” on the front to imply that this will return a boolean. In better languages, such as Ruby, you can add a question mark to the function to imply that it returns a boolean (isDannyDevito?())

// AUTHOR: Elongo Wendrico
function isDetailsEmpty($detailMessages)
{
    return isEmpty($detailMessages);
}

After that, you may need to drink a cool glass of water and listen to some jazz to relax because this next opinion is likely to raise your heart rate:
A comment to show the author? Seriously?!
Who.👏 Gives.👏 A.👏 Fuck.
Get this amateur graffiti out of your codebase and run ‘git blame’ if you want to see the author.

First of all, your code should not be dependent on the presence of one particular person to describe it. The team should know what that code does, and the best way for that to happen is to have the code describe itself so that anyone can jump into your project and quickly understand what is being achieved. Any of this ‘updated at’, ‘written by’ nonsense becomes superfluous with the use of ‘git blame’, and ‘ls -al’.
Don’t get wrapped up in pride over the code, either, because it only takes about a year or 2 before you look back on it in shame. Do you honestly want your name attached to something like the code demonstrated in this post? Do you really want to live with the thought that someone is currently cursing your decisions and writing blog posts over your small spelling errors and choice of variable names? (like $max_cnt, which describes the persona I use when writing blog posts)

function isDetailsEmpty($detailMessages)
{
    return isEmpty($detailMessages);
}

I am saying get rid of comments. Unless you are dealing with 2000 lines of jQuery on a single page, I really, truly, honestly think that you don’t need comments in your code. (except maybe to clarify a complicated regular expression or give information about something very weird) Your code should tell the story of what it does. Additionally, comments aren’t DRY; you have your code as well as your comments, so what happens if the code changes?… you need to update those comments.

I hope you went through this post, looking for the smallest fuckups in all of the detailes, because that’s your job when you are reviewing code. You want everything to be perfect because if it is not perfect, you have yourself a malignant piece of trash that could snowball into full-blown technical debt unless you take care of it now.

After everything is done, we have a function that performs a piece of core functionality from PHP. We didn’t even need to create another function for this, and can reduce it to just isEmpty()

isEmpty($detailMessages);

Write better code.

Leave a Reply

Your email address will not be published.