JavaScript Code Smells — Functions

John Au-Yeung - Apr 2 '20 - - Dev Community

Subscribe to my email list now at http://jauyeung.net/subscribe/

Follow me on Twitter at https://twitter.com/AuMayeung

Many more articles at https://medium.com/@hohanga

Even more articles at http://thewebdev.info/

In programming, a code smell is a characteristic of a piece of code that indicates there may be deeper problems. It’s a subjective characteristic used for judgment of whether the code is decent quality or not by looking at it.

In this article, we’ll look at some code smells of JavaScript functions, including too many parameters, long methods, identifier length, returning too much data, and long lines of code.

Too Many Parameters

Functions can have too many parameters. If they have too many, it makes the function more complicated when reading it and calling it. It probably means that we can clean up the code in some way to make this easier to read.

For example, the following function takes many parameters:

const describeFruit = (color, name, size, price, numSeeds, type) => {  
  return `${fruitName} is ${fruitColor}. It's ${fruitSize}. It costs ${price}. It has ${numSeeds}. The type if ${type}`;  
}
Enter fullscreen mode Exit fullscreen mode

6 parameters are probably too many. We can clean this up by passing in an object instead:

const describeFruit = (fruit) => {  
  return `${fruit.name} is ${fruit.color}. It's ${fruit.size}. It costs ${fruit.price}. It has ${fruit.numSeeds}. The type if ${fruit.type}`;  
}
Enter fullscreen mode Exit fullscreen mode

As we can see, it’s much cleaner. We don’t have to worry about passing in many arguments.

It also fits better on the screen since it’s shorter.

5 parameters are probably the maximum that should be in a function.

Excessively Long Identifiers

Identifiers that are too long are hard to read. If they can be shorter without losing any information then make them shorter.

For example, we can shorten the following variable declaration:

let colorOfAFruit = 'red';
Enter fullscreen mode Exit fullscreen mode

into:

let fruitColor = 'red';
Enter fullscreen mode Exit fullscreen mode

by removing the Of and A to make it shorter. It doesn’t change the meaning or remove any information. However, it’s shorter so we type less and get the same results.

Excessively Short Identifiers

Identifiers that are too short are also a problem. This is because identifiers that are too short don’t capture all the meaning of the entity that we define.

For example, the following variable name is too short:

let x = 'red';
Enter fullscreen mode Exit fullscreen mode

In the code above, x is too short since we have no idea what it means by looking at the variable name.

Instead, we should write:

let fruitColor = 'red';
Enter fullscreen mode Exit fullscreen mode

So that we know the variable is the color of a fruit.

Excessive Return of Data

Functions that return data we don’t need isn’t good. A function should only return what’s needed by outside code so that we don’t expose extra stuff that isn’t needed.

For example, if we have the following function:

const getFruitColor = (fruit) => {  
  return {  
    ...fruit,  
    size: 'big'  
  };  
}
Enter fullscreen mode Exit fullscreen mode

We have getFruitColor function with the size property, which isn’t relevant to the color of the fruit. Therefore, it isn’t needed and shouldn’t be returned with the object.

Instead, we should return a string with the fruit color as follows:

const getFruitColor = (fruit) => fruit.color;
Enter fullscreen mode Exit fullscreen mode

The code above is much cleaner and only returns the fruit color as suggested by the name of the function.

Excessively Long Line of Code

Lines of code that are too long make the codebase hard to read, understand and debug.

Code that is so long that they don’t fit in the screen probably should be broken into multiple lines.

For example, instead of writing:

$("p").html("This is a paragraph").css("color", "red").css("text-align", "center").slideUp(1000).slideDown(1000);
Enter fullscreen mode Exit fullscreen mode

We should instead write:

$("p")  
  .html("This is a paragraph")  
  .css("color", "red")  
  .css("text-align", "center")  
  .slideUp(1000)  
  .slideDown(1000);
Enter fullscreen mode Exit fullscreen mode

This is much cleaner and doesn’t overflow the screen.

Each line of code shouldn’t be more 100 characters so that they can be read without scrolling on any screen.

Code formatters can rearrange the lines so that they’re shorter in most cases.

Conclusion

Having too many parameters in a method makes passing in data hard since it’s easy to miss some items. It also makes the method signature excessively long.

Identifiers should just be long enough to identify the information we need. Also, it shouldn’t be so short that we don’t get enough information from the identifier.

A function should only return the items that we need and no more. This makes using the function easy since there’s less data to handle and not expose extra information that we don’t want to expose.

Finally, long lines of code should be broken into multiple lines so that they’re easier to read and change. Code formatters can break code into multiple lines automatically.

. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .