Okay codes
Riku Rouvilahello@rikurouvila.fi +358504700715CVGitHub icon
Published on Nov 1st 2018

Define value boundaries early, keep things flat

This is actually one of my personal favourite refactors. It reduces nesting, gives structure for functions and in many cases provides the answer to readers' question quicker.

From this:

function upload(file) {
  if (file.size < 9999999) {
    /* 1. */
    if (file.name.length > 5) { 
      if (file.format === 'jpeg') {
        saveFile('jpegs/' + file.name);
      /* 2. */
      } else {
        saveFile('others/' + file.name);
      }
    }
  }
}

To this:

function upload(file) {
  if (file.size >= 9999999) {
    return;
  }
  
  if (file.name.length <= 5) {
    return;
  }

  if (file.format === 'jpeg') {
    saveFile('jpegs/' + file.name);
    return;
  }
  
  saveFile('others/' + file.name);
}

Explanation

1. Always avoid nesting if statements

In most cases it's not actually needed. Oftentimes it's a sign that conditions should be either flipped (take a look at what happens to if (file.size < 9999999)) or combined.

1.1. Define parameter boundaries early, maximise happy code

Notice also that by doing this, we are able to draw a line between dangerous code, where we're unsure about the validity of our parameters and the happy code where we know the input is always valid. Happy code is easier to both read and write and we aim to maximise the amount of it.

1.2. Validate as soon as you can

In this example, we would ideally want to validate the file parameter before it hits this function. That way we could drop the if statements altogether. We could do this for example in the function calling this function. Or even the function calling that one. Ideally we wouldn't have invalid files in our application at all!

👍 As a rule of thumb: Validate user inputted parameters as soon as they reach your code.

function upload(file) {
-   if (file.size < 9999999) {
-     /* 1. */
-     if (file.name.length > 5) { 
-       if (file.format === 'jpeg') {
-         saveFile('jpegs/' + file.name);
-       /* 2. */
-       } else {
-         saveFile('others/' + file.name);
-       }
-     }
+   if (file.size >= 9999999) {
+     return;
+   }
+   
+   if (file.name.length <= 5) { 
+     return
+   }
+   if (file.format === 'jpeg') {
+     saveFile('jpegs/' + file.name);
+   /* 2. */
+   } else {
+     saveFile('others/' + file.name);
  }
}
function upload(file) {
  if (file.size >= 9999999) {
    return;
  }
  
  if (file.name.length <= 5) { 
    return
  }
  if (file.format === 'jpeg') {
    saveFile('jpegs/' + file.name);
  /* 2. */
  } else {
    saveFile('others/' + file.name);
  }
}

2. else is often unnecessary

In this case getting rid of else by returning from the first branch gets rid of 1 level of indentation. Some linters also complain about this, because the code will be unreachable

function upload(file) {
  if (file.size >= 9999999) {
    return;
  }
  
  if (file.name.length <= 5) { 
    return
  }
  
  if (file.format === 'jpeg') {
    saveFile('jpegs/' + file.name);
- } else {
-   saveFile('others/' + file.name);
+   return;
  }
+   
+  saveFile('others/' + file.name);
}

Why I say it's often unnecessary is, that there are cases where it can be argued using else improves readability.

Consider:

if (user) {
  res.send(200)
} else {
  res.send(404)
}

vs

if (user) {
  res.send(200)
  return
} 
res.send(404)

Which one do you prefer? The latter one indeed saves you one indent, but adds a return statement that only has the purpose of stopping the function from running. (credits to @Cadiac for this)


I love feedback!

First of all: thank you so much for reading!
If you have something to ask, don't agree with something I say or just wanna have a chat, feel free to ping me on Twitter.

Freelance Software Developer in Tampere, FinlandRiku Rouvila
Software developer