loading...
Cover image for 1 small tip to improve your code readability

1 small tip to improve your code readability

robertotonino profile image Roberto Tonino Updated on ・1 min read

(Kevin Du photo on Pexels)

Code readability is important.

When you find yourself in situations like this one:

function doSomething() {
  // some code...
  let needToDoALotOfThings = /* test */

  if (needToDoALotOfThings) {
    /*

       A good amount of code

    */
  }
}

you can refactor it in this way:

function doSomething() {
  // some code...
  let needToDoALotOfThings = /* test */

  if (!needToDoALotOfThings) return

  /*

     A good amount of code

  */

}

or, even better:

function doSomething() {
  // some code...
  let needToDoALotOfThings = /* test */

  if (!needToDoALotOfThings) throw new Error(/* error message */)

  /*

     A good amount of code

  */

}

The difference is slight but significant. By using this approach, you'll have (at least) 2 advantages:

  1. 1 less indentation level, that is always good;
  2. Your condition is shrinked in 1 line of code, making the code easier to read in future reviews.

You obviously can't use this approach everywhere, it depends on the situation (as always), but it's a small correction that can save a bit of brain cells to the person that will read that snippet of code in the future.

Discussion

pic
Editor guide
 

I love guard clauses like these! Thanks for the article

They're especially helpful when multiple conditions should be checked (aka a "preflight checklist")

Compare:

function doSomething(a,b) {
  if(a && a.type == "correct" && b && b.type == "correct" && a.timestamp >= b.timestamp)
  {
    /* ... */
  }
}

to the guard version:

function doSomething(a, b) {
  if (!a) return;
  if (a.type != "correct") return;
  if (!b) return;
  if (b.type != "correct") return;
  if (a.timestamp < b.timestamp) return;
  /* ... */
}

The guard version:

  1. has less indention
  2. has more separation of logic
  3. minimizes diffs in line based version control (aka git) when changing conditions

(and in a language like Ruby, something I particularly like is if(!a) return guard clauses can be rewritten as return unless a)

 

Optional Channing can use reduce the lines i believe 🤔

 

And... Follow. Fail fast is an amazing mindset. Thank you for using your voice to empower writing good code.

 

Doesn't fail fast refer to asserting preconditions rather than silently doing nothing?

 

You’re right, silence is always bad. Maybe I’m just too much used to it because of the codebase I work with. Gonna update the article with a throw. Thanks

 

What I gain from the post is that flipping your conditionals can lead to more readable code. There are lots of cases where this is very very true.

Silently doing nothing isn't good, but neither is dragging out your code by putting the "fast" case in the else statement.

 

Thank you man, being following Uncle Bob lessons lately and wow! Mind opened.

 

Why call a method if it does nothing? Sure if you are protecting the boundaries of the application to an external interface you need to guard the boundary but even then you should collate consolidated error stats (accounting for potential security/dos events if relevant). If the code is not protecting the system from external systems wouldn't these conditions be better reflected in internal code as exceptions or assertions i.e. preconditions? Even in change detection algorithms I would comment the else branch to say "no change has been detected so do nothing". Commenting the else branches makes it clear you have considered all cases. If I find I am starting to nest too deeply, I use it as a trigger to refactor.

 

I think there are certain cases where this is really useful though. For instance, writing a publicly used library or public API. If a developer doesn't know the internals of your project, they shouldn't be expected to call it correctly the first time, right?

 

If the API code simply does nothing when a user of the API does not satisfy preconditions, how can the API user figure out they are not using it correctly? Wouldn't an exception be more appropriate?

Totally, but all too often I see things like,
If (acceptable)
{
// Lots of code
}
Else
{
Throw exception
}

Vs what the article talks about of

If (!acceptable)
Throw exception

 

When working with animations and rendering. I found this pattern pretty useful, usually the first thing that you do in your rendering is to detect if something has changed, if nothing has changed then do nothing and skip to the next call.

 
 

no it is not, here is only a tip for avoiding deep nested indentations.
When I started coding I also buy into the singe exit/return theory.
but i think the tip of exit/return early in this article is very good.

JS since 2015 also had tail call optimization, it changes a call operation into a jump operation, that is more 3ffective for the CPU. It also allowes deeper callstack sizes, as there is no more call. js has a max callsize deeped of about 16000. some features could not be written in recursive coding style, but today it can.

 

Oldstyle folks (like me 😀) remember this from Uncle's Bob “Refacroring” book. The technique is called “Replacing nested conditionals with guard clauses”

 

This kind of coding practice also called Bouncer Pattern.