Common Code Smells and Heuristics
Before we start learning about code issues, improvements, and rules, let’s see how many IT professionals worldwide rely on code, whether they write it themselves or not (like managers, for example).
As per some stats:
- There are an estimated 26.9 million professional software engineers in the world as of 2022
- There are nearly 27 million developers worldwide. This number is expected to exceed 28.7 million by 2024
- According to industry research firm Janco Associates, more than 4.18 million people are employed as IT professionals in the US
Now, we will go through the section wise code smells and heuristics.
Comments:
Inappropriate information: Don’t put info in comments that should be in other systems like source code control or issue tracking. Comments are for technical notes about the code and design, not for things like change histories or metadata.
/**
* Author: Jane Doe
* Last Modified: 2022-04-20
* Version: 1.2
* Description: This class represents a basic calculator.
* Change History:
* - Version 1.0: Initial implementation
* - Version 1.1: Fixed a bug in the add() method
* - Version 1.2: Added support for subtraction
*/
public class Calculator {
// Calculate the sum of two numbers
public int add(int a, int b) {
return a + b;
}
// Calculate the difference between two numbers
public int subtract(int a, int b) {
return a - b;
}
}In the text above, it’s saying that the information about changes, history, and when something was last modified is written in a comment. But this information isn’t necessary because it’s already stored in a source code control system like Git.
Cleaner version:
/**
* This class represents a basic calculator.
*/
public class Calculator {
// Calculate the sum of two numbers
public int add(int a, int b) {
return a + b;
}
// Calculate the difference between two numbers
public int subtract(int a, int b) {
return a - b;
}
}Obsolete comment: Obsolete comments, being outdated, should be avoided. Remove or update them promptly to prevent confusion. Sometime we change the code but not the comments :)
public class ObsoleteCommentsExample {
// Calculate the sum of two numbers
public int add(int a, int b) {
return a + b;
}
// Author: John Smith
// Last Modified: 2020-12-05
// This method is used to add two numbers.
// Change History:
// - Version 1.0: Initial implementation
// - Version 1.1: Fixed a bug in the addition logic (Obsolete)
public int addObsolete(int a, int b) {
// Obsolete comment: The bug was fixed in version 1.1, but the comment was not updated.
return a + b;
}
// Calculate the difference between two numbers
public int subtract(int a, int b) {
return a - b;
}
// Description: This method subtracts two numbers.
// Change History:
// - Version 1.0: Initial implementation
public int subtractObsolete(int a, int b) {
// Obsolete comment: The comment contains unnecessary information and is outdated.
return a - b;
}
}Redundant comment: A comment is redundant if it describes something that adequately describes itself.
i++; // increment iComments should say things that the code cannot say for itself, example:
/**
* @param sellRequest
* @return
* @throws ManagedComponentException
*/
public SellResponse beginSellItem(SellRequest sellRequest)
throws ManagedComponentExceptionPoorly Written Comment: A well-written comment is a valuable addition. When you choose to include a comment, take the opportunity to create a high-quality one. Pay attention to word choice, maintain proper grammar and punctuation, avoid excessive elaboration, and keep it succinct.
public class PoorCommentExample {
// This is a function that adds two numbers
public int addition(int num1, int num2) {
// Here, we add num1 and num2 together
int result = num1 + num2; // The result is stored in the result variable
return result; // We return the result
}
}public class CommentQualityExample {
// This method calculates the sum of two numbers.
// It takes two integers as input and returns their sum.
public int add(int a, int b) {
// Here, we add the two numbers and store the result in 'sum'.
int sum = a + b;
// Finally, we return the 'sum' as the result of the addition.
return sum;
}
// This method finds the difference between two numbers.
// It subtracts 'b' from 'a' and returns the result.
public int subtract(int a, int b) {
// To calculate the difference, we subtract 'b' from 'a'.
int difference = a - b;
// We then return the 'difference' as the result.
return difference;
}
// This method multiplies two numbers.
public int multiply(int a, int b) {
// Multiply 'a' and 'b' to get the result.
int result = a * b;
// Return the 'result'.
return result;
}
}Commented out code: This issue frequently arises during code reviews. When questioned, developers often offer excuses like “someone else did it” or “we might need it later.” However, as a firm guideline, it’s best to eliminate commented-out code. You can always retrieve it from the version control system when necessary.
public class CommentedCodeExample {
public static void main(String[] args) {
int x = 5;
int y = 10;
// int result = x + y;
// System.out.println("The sum is: " + result);
// The code below is no longer needed.
// It was used for an earlier feature that has been removed.
// int z = 20;
// int finalResult = result + z;
// System.out.println("The final result is: " + finalResult);
}
}Environment:
Build Requires More Than One Step: Starting a project should be easy. You shouldn’t need to fetch many code pieces, use complex commands, or hunt for extra files. Just one command to check out the system and one more to build it should suffice.
# Before applying simplification heuristic
git clone <repository-url>
cd project
./setup.sh
./build.sh
# Manually find and copy required JAR files
# Search for and configure XML files from different directories# After applying simplification heuristic
git clone <repository-url>
./build.shTests Require More Than One Step: Running all unit tests should be simple. You should be able to do it with one click in your IDE or with a single command in your shell. It’s a fundamental and essential task, so it should be quick and easy.
# Before streamlining testing
cd project-directory
./setup-environment.sh
./run-unit-tests.sh# After streamlining testing
cd project-directory
./run-tests.shFunctions:
Too Many Arguments: Functions are most effective when they accept a modest number of arguments. The ideal scenario is having zero arguments, followed by one, two, and three. If a function has more than three arguments, it becomes problematic and should be minimized whenever possible. This is because an excessive number of arguments often indicates that the function is handling multiple tasks, which goes against the Single Responsibility Principle (SRP).
public double calculateTotalPrice(double item1, double item2, double item3, double item4, double item5, double item6) {
double total = item1 + item2 + item3 + item4 + item5 + item6;
return total;
}public double calculateTotalPrice(double[] items) {
double total = 0.0;
for (double item : items) {
total += item;
}
return total;
}Output Arguments: Output arguments can be confusing. People usually think of arguments as things you put into a function, not things that come out of it. If your function needs to modify something, it’s better to have it change the thing it’s working on, like an object it’s called on.
public void appendToList(int item, List<Integer> outputList) {
// Try to add the item to the output list
outputList.add(item);
}public List<Integer> createNewListWithItem(int item, List<Integer> inputList) {
List<Integer> newList = new ArrayList<>(inputList);
newList.add(item);
return newList;
}Flag Arguments: When a method takes a flag as an argument, it often suggests an attempt to perform multiple tasks within the same method. This typically requires the use of conditional statements (if/else) to determine which task to execute.
public double calculateTotalPrice(List<Item> items, boolean applyDiscount) {
double totalPrice = 0.0;
for (Item item : items) {
if (applyDiscount) {
totalPrice += item.getDiscountedPrice();
} else {
totalPrice += item.getPrice();
}
}
return totalPrice;
}public double calculateTotalPriceWithDiscount(List<Item> items) {
double totalPrice = 0.0;
for (Item item : items) {
totalPrice += item.getDiscountedPrice();
}
return totalPrice;
}
public double calculateTotalPriceWithoutDiscount(List<Item> items) {
double totalPrice = 0.0;
for (Item item : items) {
totalPrice += item.getPrice();
}
return totalPrice;
}Dead Function: A method that is never called in any part of the code should be promptly removed from the codebase.
public class Calculator {
public int add(int a, int b) {
return a + b;
}
// This method is never called
public int subtract(int a, int b) {
return a - b;
}
}public class Calculator {
public int add(int a, int b) {
return a + b;
}
}- Code coverage tools like JaCoCo (for Java), Istanbul (for JavaScript), and Coverage.py (for Python) can help identify code that is not executed by tests, which often indicates dead code.
- SonarQube: SonarQube is a widely used code quality platform that can identify dead code and provide detailed reports on code quality.
- FindBugs: FindBugs is a static analysis tool for Java that can detect various issues, including dead code.
- ESLint and TSLint: For JavaScript and TypeScript projects, ESLint and TSLint can identify unused variables and functions.
