From Messy Code to Masterpiece: The Art of Refactoring and Code Smell Removal
“If it stinks, change it!! “— Clean Code
What is Code Smell?
Code Smell refers to certain patterns or characteristics in code that indicate potential design or implementation issues. They are not bugs but rather indicators of poor code quality that can make the code harder to understand, maintain, and extend.
What is Refactoring?
Refactoring, on the other hand, is the process of restructuring existing code without changing its external behavior. It aims to improve the design, readability, and maintainability of the codebase while preserving the functionality. Refactoring involves making small, incremental changes to the code to eliminate code smells and improve its overall quality.
By converting code smells into refactoring, several benefits can be achieved:
- Improved Readability: Refactoring helps in making the code easier to read and understand. By eliminating code smells, the code becomes more self-explanatory and follows established coding conventions.
- Increased Maintainability: Refactoring reduces complexity, eliminates duplication, and improves the organization of code. This makes it easier to modify and maintain the codebase over time, reducing the chances of introducing bugs during future changes.
- Enhanced Extensibility: Refactoring promotes a modular and flexible code structure. By eliminating code smells, the codebase becomes more extensible, allowing for easier addition of new features or modifications without disrupting the existing functionality.
- Better Collaboration: Clean and well-structured code enhances collaboration among team members. Refactoring helps in improving code quality, making it easier for multiple developers to work on the same codebase efficiently and understand each other’s contributions.
- Increased Testability: Refactored code is typically easier to test. By eliminating code smells, the codebase becomes more modular and decoupled, allowing for more focused and comprehensive testing.
Common Code Smells and how to remove the Code Smell!!
1. Inappropriate Naming
Code smells related to naming involve using misleading, unclear, or non-descriptive names for variables, methods, classes, etc. It makes the code harder to understand and maintain.
Example:
public class P {
public int a;
public void s(int x) {
a = x;
System.out.println(a);
}
}Refactoring Steps:
- Rename the class to a more descriptive name, like
Printer - Rename the variable
ato a meaningful name, likenumber - Rename the method
sto a more descriptive name, likeprintNumber
public class Printer {
public int number;
public void printNumber(int x) {
number = x;
System.out.println(number);
}
}2. Long Method
Long methods are overly lengthy and complex, making them harder to understand, test, and maintain.
Generally, any method longer than ten lines should make you start asking questions.
public double calculateTotalPrice(List<Item> items) {
double totalPrice = 0.0;
// Task 1: Iterate over the items and calculate the total price
for (Item item : items) {
// Task 2: Calculate the item price
double itemPrice = item.getPrice();
// Task 3: Add the item price to the total
totalPrice += itemPrice;
}
// Task 4: Apply any discounts
if (totalPrice > 100.0) {
totalPrice -= 10.0;
}
// Task 5: Apply any additional taxes
totalPrice *= 1.1;
return totalPrice;
}Refactoring Steps:
- Extract code into smaller, self-contained methods with descriptive names.
- Identify logical sections within the method and move them into separate methods.
- Pass necessary parameters to the extracted methods and return any required values.
public double calculateTotalPrice(List<Item> items) {
double totalPrice = 0.0;
for (Item item : items) { // Task - 1
double itemPrice = calculateItemPrice(item); // Task - 2
totalPrice += itemPrice; // Task - 3
}
totalPrice = applyDiscounts(totalPrice); // Task - 4
totalPrice = applyTaxes(totalPrice); // Task - 5
return totalPrice;
}// Task - 2
private double calculateItemPrice(Item item) {
double itemPrice = item.getPrice();
// Additional calculations for item price if necessary
return itemPrice;
}// Task - 4
private double applyDiscounts(double totalPrice) {
if (totalPrice > 100.0) {
totalPrice -= 10.0;
}
return totalPrice;
}// Task - 5
private double applyTaxes(double totalPrice) {
totalPrice *= 1.1;
return totalPrice;
}
3. Comments
Excessive or misleading comments can indicate unclear code or a lack of self-explanatory code.
public class Calculator {
public int add(int a, int b) {
// Adding two numbers
return a + b;
}
}Refactoring Steps: Remove unnecessary comments.
public class Calculator {
public int add(int a, int b) {
return a + b;
}
}
4. Dead Code
Dead code refers to unreachable or unused code segments that serve no purpose and should be removed.
public void calculateTotal(int quantity, double price) {
double subtotal = quantity * price;
double tax = calculateTax(subtotal);
double total = subtotal + tax;
System.out.println("Total: " + total);
int unusedVariable = 5; // Unused variable
}Refactoring Steps:
- Identify any code segments that are not used or have no effect on the program’s behavior.
- Remove the dead code: (int unusedVariable = 5;)
public void calculateTotal(int quantity, double price) {
double subtotal = quantity * price;
double tax = calculateTax(subtotal);
double total = subtotal + tax;
System.out.println("Total: " + total);
}5. Duplicate Code
Duplicate code occurs when the same or very similar code is repeated in multiple places, leading to maintenance difficulties.
public class Calculator {
public double calculateCircleArea(double radius) {
double area = Math.PI * radius * radius;
System.out.println("The area of the circle is: " + area);
return area;
}
public double calculateRectangleArea(double length, double width) {
double area = length * width;
System.out.println("The area of the rectangle is: " + area);
return area;
}
public double calculateTriangleArea(double base, double height) {
double area = 0.5 * base * height;
System.out.println("The area of the triangle is: " + area);
return area;
}
}In the above example, the code for printing the area is duplicated across the calculateCircleArea, calculateRectangleArea, and calculateTriangleArea methods.
Refactoring Step:
public class Calculator {
public double calculateCircleArea(double radius) {
double area = Math.PI * radius * radius;
printArea("circle", area);
return area;
}
public double calculateRectangleArea(double length, double width) {
double area = length * width;
printArea("rectangle", area);
return area;
}
public double calculateTriangleArea(double base, double height) {
double area = 0.5 * base * height;
printArea("triangle", area);
return area;
}
// refactoring the code using printArea()
private void printArea(String shape, double area) {
System.out.println("The area of the " + shape + " is: " + area);
}
}
6. Primitive Obsession
Primitive Obsession is a code smell that occurs when primitive data types, such as integers or strings, are overused to represent concepts or entities instead of creating dedicated classes.
Use classes and object to encapsulate the behavior and data!
public class Person {
private String name;
private int age;
private String contactNumber;
public boolean isValidContactNumber() {
// contact number validation logic
return contactNumber.matches("\\d{10}");
}
public boolean isAdult() {
return age >= 18;
}
}Refactoring Steps:
- Identify primitive types that can be encapsulated in separate classes.
- Create small, domain-specific classes for those primitives.
public class Person {
private String name;
private ContactNumber contactNumber;
private Age age;
public boolean isValidContactNumber() {
return contactNumber.isValid();
}
public boolean isAdult() {
return age.isAdult();
}
}// Contract Number Class instead of String Variable
public class ContactNumber {
private String value;
public ContactNumber(String value) {
this.value = value;
}
public boolean isValid() {
// contact number validation logic
return value.matches("\\d{10}");
}
}// Age Class instead of int Variable
public class Age {
private int value;
public Age(int value) {
this.value = value;
}
public boolean isAdult() {
return value >= 18;
}
}7. Large Class
A large class is one that has grown too big, contains too many responsibilities, and violates the Single Responsibility Principle.
public class BankService {
public void login(String userName , String password){
// authenticate user
}
public long deposit(long amount, String accountNo) {
//deposit amount
return 0;
}
public long withDraw(long amount, String accountNo) {
//withdraw amount
return 0;
}
}Refactoring Steps:
- Identify cohesive groups of functionality within the large class.
- Extract those groups into separate classes, following the Single Responsibility Principle.
public class BankServiceLogin{
public void login(String userName , String password){
// authenticate user
}
}public class BankServiceWithdraw {
public long withDraw(long amount, String accountNo) {
//withdraw amount
return 0;
}
}public class BankServiceDeposit {
public long deposit(long amount, String accountNo) {
//deposit amount
return 0;
}
}8. Lazy Class
A lazy class is a class that doesn’t have enough responsibilities or functionality to justify its existence. It adds unnecessary complexity to the codebase.
public class Employee {
// ... only contains basic getters and setters
}Refactoring Steps:
- Identify the responsibilities or functionality that can be moved out of the lazy class.
- Distribute those responsibilities to other classes, merging or eliminating the lazy class if necessary.
// Remove the Employee class
// distributed its responsibilities to other relevant classes.
9. Long Parameter List
A long parameter list occurs when a method has too many parameters, which can make the code harder to read and maintain. More than three or four parameters for a method.
public void createOrder(String customerName,
String customerAddress,
String productName,
int quantity,
double price) {
// ... implementation
}Refactoring Steps:
- Identify related parameters that can be grouped together into meaningful objects.
- Create a class to encapsulate those grouped parameters.
public class Order {
private String customerName;
private String customerAddress;
private String productName;
private int quantity;
private double price;
// ... constructor, getters, setters, and other relevant methods
}
public void createOrder(Order order) {
// ... implementation
}10. Switch Statement
The switch statement code smell occurs when there is an excessive use of switch or case statements.
public class ShapePrinter {
public void printShape(String shape) {
switch (shape) {
case "rectangle":
printRectangle();
break;
case "circle":
printCircle();
break;
case "triangle":
printTriangle();
break;
default:
throw new IllegalArgumentException("Invalid shape");
}
}
private void printRectangle() {
System.out.println("Printing rectangle shape");
// Logic to print a rectangle
}
private void printCircle() {
System.out.println("Printing circle shape");
// Logic to print a circle
}
private void printTriangle() {
System.out.println("Printing triangle shape");
// Logic to print a triangle
}
}Refactoring Steps:
- Identify the behavior represented by the switch statement that can be encapsulated into separate classes or methods.
- Create a polymorphic structure using inheritance or interfaces to handle different cases without switch statements.
public abstract class Shape {
public abstract void print();
}public class Rectangle extends Shape {
@Override
public void print() {
System.out.println("Printing rectangle shape");
// Logic to print a rectangle
}
}public class Circle extends Shape {
@Override
public void print() {
System.out.println("Printing circle shape");
// Logic to print a circle
}
}public class Triangle extends Shape {
@Override
public void print() {
System.out.println("Printing triangle shape");
// Logic to print a triangle
}
}// removed switch cases from ShapePrinter using inheritance
public class ShapePrinter {
public void printShape(Shape shape) {
shape.print();
}
}11. Speculative Generality
Speculative Generality occurs when code is designed to be overly generic or flexible for potential future needs that may never arise. It adds unnecessary complexity and maintenance overhead.
public class Utils {
public static <T> void printList(List<T> list) {
for (T item : list) {
System.out.println(item);
}
}
}Refactoring Steps:
- Identify the generic or flexible code that is not currently needed.
- Remove the unnecessary generic code and simplify the design.
public class Utils {
public static void printList(List<String> list) {
for (Object item : list) {
System.out.println(item);
}
}
}12. Oddball Solution
Oddball Solution refers to a non-standard or unconventional way of solving a problem. It deviates from standard coding practices and can confuse other developers.
public class MathUtils {
public static int add(int a, int b) {
return a - (-b); // Code Smell
}
}Refactoring Steps:
- Identify the non-standard or unconventional solution.
- Refactor the code to use standard, well-accepted coding practices and conventions.
public class MathUtils {
public static int add(int a, int b) {
return a + b;
}
}
13. Feature Envy
Feature Envy occurs when a method excessively uses data or behavior from another class, indicating that it might belong more naturally to that other class. It leads to less maintainable and less cohesive code.
public class Customer {
private String name;
private int age;
private ShoppingCart shoppingCart;
public void displayShoppingCartTotal() { // Feature Envy Child
double total = 0.0;
for (Item item : shoppingCart.getItems()) {
total += item.getPrice();
}
System.out.println("Total amount in shopping cart: " + total);
}
}
public class ShoppingCart {
private List<Item> items;
//Feature Envy Parent
public List<Item> getItems() {
return items;
}
}
public class Item {
private String name;
private double price;
public double getPrice() {
return price;
}
}n this example, the displayShoppingCartTotal() method in the Customer class exhibits Feature Envy. It accesses the items list from the ShoppingCart class excessively to calculate the total price.
Refactoring Steps:
- Identify the methods in the current class that heavily use data or behavior from another class.
- Move those methods to the class that owns the data or behavior.
To refactor this code, we can move the responsibility of calculating the total amount to the ShoppingCart class itself. This can be done by introducing a new method, such as getTotalPrice(), in the ShoppingCart class.
public class Customer {
private String name;
private int age;
private ShoppingCart shoppingCart;
public void displayShoppingCartTotal() {
double total = shoppingCart.getTotalPrice(); // Step - 1
System.out.println("Total amount in shopping cart: " + total);
}
}
public class ShoppingCart {
private List<Item> items;
// Step - 2
public double getTotalPrice() {
double total = 0.0;
for (Item item : items) {
total += item.getPrice();
}
return total;
}
}
public class Item {
private String name;
private double price;
// Constructor and other methods
public double getPrice() {
return price;
}
}14. Refused Bequest
Refused Bequest occurs when a subclass inherits from a superclass but doesn’t use or fulfill the contracts and expectations of the inherited methods.
Liskov Substitution Principle
public class Bird {
public void fly() {
System.out.println("I'm flying");
}
public void walk() {
System.out.println("I'm walking");
}
}
public class Dove extends Bird{
public void fly() {
System.out.println("I'm flying");
}
public void walk() {
System.out.println("I'm walking");
}
}
// Wrong Inhertance from super class
public class Penguin extends Bird{
public void fly() {
throw new IllegalArgumentException("Can't Fly");
}
public void walk() {
System.out.println("I'm walking");
}
}Refactoring Steps:
- Identify the inherited methods in the subclass that are not used or don’t fulfill their contracts.
- Either override those methods in the subclass and provide appropriate behavior or remove them if they are unnecessary.
public class Bird {
public void walk(){
System.out.println("I'm walking");
}
}
public class FlyingBird extends Bird{
public void fly(){
System.out.println("I'm flying");
}
}
public class Dove extends FlyingBird{}{
// dove can fly and walk
}
public class Penguin extends Bird{
// penguin can't fly but can walk
}
15. Black Sheep
Black Sheep refers to a class or method that doesn’t fit the overall design and purpose of the system. It stands out as different and doesn’t follow the established patterns and conventions.
Follows Single Responsibility Principle
public class Bank {
private String name;
private List<Account> accounts;
public Bank(String name) {
this.name = name;
accounts = new ArrayList<>();
}
public void addAccount(Account account) {
accounts.add(account);
}
public void transferMoney(Account sourceAccount, Account targetAccount, double amount) {
if (sourceAccount.getBalance() >= amount) {
sourceAccount.setBalance(sourceAccount.getBalance() - amount);
targetAccount.setBalance(targetAccount.getBalance() + amount);
System.out.println("Money transferred successfully.");
} else {
System.out.println("Insufficient balance.");
}
}
public void printAccountDetails(Account account) {
System.out.println("Account Holder: " + account.getHolderName());
System.out.println("Account Number: " + account.getAccountNumber());
System.out.println("Balance: " + account.getBalance());
}
}In this example, the Bank class exhibits the Black Sheep code smell. It combines different responsibilities, such as managing accounts, transferring money, and printing account details, within the same class
Refactoring Steps:
- Identify the class or method that doesn’t fit the overall design.
- Either refactor the class or method to align with the system’s design or remove it if it’s unnecessary
public class Bank {
private String name;
private List<Account> accounts;
public Bank(String name) {
this.name = name;
accounts = new ArrayList<>();
}
public void addAccount(Account account) {
accounts.add(account);
}
public void transferMoney(Account sourceAccount, Account targetAccount, double amount) {
MoneyTransferService transferService = new MoneyTransferService();
transferService.transferMoney(sourceAccount, targetAccount, amount);
}
public void printAccountDetails(Account account) {
AccountDetailsPrinter detailsPrinter = new AccountDetailsPrinter();
detailsPrinter.printAccountDetails(account);
}
}
public class MoneyTransferService {
public void transferMoney(Account sourceAccount, Account targetAccount, double amount) {
if (sourceAccount.getBalance() >= amount) {
sourceAccount.setBalance(sourceAccount.getBalance() - amount);
targetAccount.setBalance(targetAccount.getBalance() + amount);
System.out.println("Money transferred successfully.");
} else {
System.out.println("Insufficient balance.");
}
}
}
public class AccountDetailsPrinter {
public void printAccountDetails(Account account) {
System.out.println("Account Holder: " + account.getHolderName());
System.out.println("Account Number: " + account.getAccountNumber());
System.out.println("Balance: " + account.getBalance());
}
}SOLID principle with Code Smell Refactoring
Single Responsibility Principle (SRP):
- Large Class: A large class with multiple responsibilities violates SRP as each class should have a single reason to change.
- Long Method: Long methods that perform multiple tasks can indicate a violation of SRP as each method should have a single responsibility.
- Long Parameter List: A long parameter list can suggest that a method is responsible for multiple tasks, violating SRP.
- Primitive Obsession: Primitive Obsession occurs when primitive types are used instead of creating small, domain-specific classes. It can violate SRP if a class takes on responsibilities beyond its primary purpose.
Open/Closed Principle (OCP):
- Switch Statement: Excessive use of switch statements can violate the OCP as it requires modifying the code when new cases are added. It is preferable to use polymorphism or the Strategy pattern to achieve extensibility.
Liskov Substitution Principle (LSP):
- Refused Bequest: Refused Bequest occurs when a subclass inherits from a superclass but doesn’t fulfill the contracts and expectations of the inherited methods. It violates LSP as it breaks the substitutability principle.
Interface Segregation Principle (ISP):
- Lazy Class: A Lazy Class is a class that doesn’t have enough responsibilities or functionality to justify its existence. It may suggest a violation of ISP if the class implements interfaces that it doesn’t fully utilize.
Dependency Inversion Principle (DIP):
- Feature Envy: Feature Envy occurs when a method excessively uses data or behavior from another class, suggesting that it might belong more naturally to that class. It can violate DIP if it creates a strong dependency between two classes.
Level Up Coding
Thanks for being a part of our community! Before you go:
- 👏 Clap for the story and follow the author 👉
- 📰 View more content in the Level Up Coding publication
- 💰 Free coding interview course ⇒ View Course
- 🔔 Follow us: Twitter | LinkedIn | Newsletter
🚀👉 Join the Level Up talent collective and find an amazing job
