https://ajhawrites.medium.com/common-code-smells-and-heuristics-final-part-6391a095fd5f
Universal:
Inconsistency: Any code whatever we write should follow a consistent pattern, during the code review sessions. I have found people following inconsistent practices across the code. For example: inconsistent variable naming, inconsistent indentation, inconsistent spacing, incorrect method naming, inconsistent use of quotation marks, and inconsistent variable types.
public class InconsistentCodeExample {
public static void main(String[] args) {
// Inconsistent variable naming and missing semicolon
int num1 = 10;
int Num2 = 20
int result;
// Inconsistent indentation
if (num1 > Num2) {
result = num1 - Num2;
} else {
result = Num2 - num1;
}
// Inconsistent spacing and incorrect usage of the println method
System.out.println("The result is: "+result );
System.out.println("The result is: " + result);
// Inconsistent method naming
int SUM = addNumbers(num1, Num2);
int sum = AddNumbers(num1, Num2);
// Inconsistent quotation marks and missing semicolon
String message1 = "This is a valid message";
String message2 = 'This is an invalid message'
// Inconsistent variable types
int x = 5;
String y = "Hello, World!";
}
// Inconsistent method definition
public static int addNumbers(int a, int B) {
return a + B;
}
// Inconsistent method definition
public static int AddNumbers(int a, int b) {
return a + b;
}
}Clutter (H): Why have an empty default constructor? It just adds unnecessary stuff to the code — unused variables, functions that do nothing, and comments that don’t help. All of these are just clutter and should be deleted. Keep your source code neat, well-organized, and clutter-free.
public class ClutterExample {
int x; // Unused variable
public ClutterExample() {
// Empty constructor with no implementation
}
public void calculate(int a, int b) {
int result = a + b;
System.out.println("Result: " + result);
}
// Redundant comments
// This method multiplies two numbers
public int multiply(int num1, int num2) {
return num1 * num2;
}
// Unused method
public void unusedMethod() {
System.out.println("This method is never called.");
}
public static void main(String[] args) {
// Inconsistent indentation
int value = 10;
System.out.println("Value: " + value);
}
}Artificial Coupling: Don’t make things to dependent on each if that’s not required. Think about the relationship and design your classes/interfaces/methods accordingly.
public class Order {
private Inventory inventory;
public Order() {
inventory = new Inventory();
}
public void placeOrder() {
// Place the order
inventory.updateInventory();
}
}
public class Inventory {
public void updateInventory() {
// Update inventory logic
}
}public class Order {
private InventoryService inventoryService;
public Order(InventoryService inventoryService) {
this.inventoryService = inventoryService;
}
public void placeOrder() {
// Place the order
inventoryService.updateInventory();
}
}
public interface InventoryService {
void updateInventory();
}
public class Inventory implements InventoryService {
@Override
public void updateInventory() {
// Update inventory logic
}
}Feature Envy: “Feature envy” is a code smell in software development. It occurs when a section of code in one class seems to be more interested in the methods or data of another class, rather than its own.
public class HourlyPayCalculator {
public Money calculateWeeklyPay(HourlyEmployee e) {
int tenthRate = e.getTenthRate().getPennies();
int tenthsWorked = e.getTenthsWorked();
int straightTime = Math.min(400, tenthsWorked);
int overTime = Math.max(0, tenthsWorked - straightTime);
int straightPay = straightTime * tenthRate;
int overtimePay = (int)Math.round(overTime*tenthRate*1.5);
return new Money(straightPay + overtimePay);
}
}public class HourlyEmployeeReport {
private HourlyEmployee employee ;
public HourlyEmployeeReport(HourlyEmployee e) {
this.employee = e;
}
String reportHours() {
return String.format(
"Name: %s\tHours:%d.%1d\n",
employee.getName(),
employee.getTenthsWorked()/10,
employee.getTenthsWorked()%10);
}
}Selector Arguments: Avoid selector arguments, this makes your method to do multiple jobs by writing if else condition. Whenever you have to decide between a selector argument vs refactoring your method. Go for refactoring.
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;
public class SelectorArgumentsExample {
public static void main(String[] args) {
List<Integer> numbers = Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
// Using selector arguments to filter elements
List<Integer> evenNumbers = numbers.stream()
.filter(number -> number % 2 == 0) // Selector argument (lambda expression)
.collect(Collectors.toList());
System.out.println("Even numbers: " + evenNumbers);
}
}Specific to Java:
Avoid Long Import Lists by using wildcards: If you use two or more classes from a package, then import the whole package with import package.*;
Don’t Inherit Constants: Using static imports is recommended over inheriting constants. Instead of inheriting constants, it’s a best practice to use static imports.
public class Constants {
public static final int MAX_VALUE = 100;
public static final int MIN_VALUE = 0;
}import static package_name.Constants.*;
public class Example {
public static void main(String[] args) {
int value = MAX_VALUE;
System.out.println("Maximum value: " + value);
int min = MIN_VALUE;
System.out.println("Minimum value: " + min);
}
}Constants VS Enums: Using Enums over constants is recommended in many situations in Java because Enums provide several advantages, such as type safety, readability, and maintainability.
public class ConstantsExample {
public static final int RED = 0;
public static final int GREEN = 1;
public static final int BLUE = 2;
public static void main(String[] args) {
int color = GREEN; // Using a constant
if (color == RED) {
System.out.println("You selected Red.");
} else if (color == GREEN) {
System.out.println("You selected Green.");
} else if (color == BLUE) {
System.out.println("You selected Blue.");
} else {
System.out.println("Invalid color.");
}
}
}public class EnumsExample {
public enum Color {
RED, GREEN, BLUE
}
public static void main(String[] args) {
Color color = Color.GREEN; // Using an enum
switch (color) {
case RED:
System.out.println("You selected Red.");
break;
case GREEN:
System.out.println("You selected Green.");
break;
case BLUE:
System.out.println("You selected Blue.");
break;
default:
System.out.println("Invalid color.");
}
}
}Further reading: Learnings from — Clean Code: A Handbook of Agile Software Craftsmanship
Tests:
Insufficient tests: In a test suite, you might wonder how many tests you need. Some programmers rely on a vague sense of "that seems like enough." However, the key is to ensure that your test suite covers all potential points of failure. It's not enough if there are untested situations or unverified calculations. The goal is to be comprehensive and thorough in your testing.
Use a Coverage Tool: Coverage tools help you identify areas in your testing plan that may be incomplete. They highlight parts of your code that haven’t been tested well. In many code editors, you’ll see green lines for tested parts and red lines for untested parts, making it simple to spot code that you might have missed, like if statements where you haven’t checked what happens inside them.
Please read this further: A Tribute to Writers of Unorganized Test Code
