Code Smells: A Solution Architects’s Guide
Code smells are a warning sign that your code is not written in the most effective way. They can make your code harder to read, understand, and maintain. In this blog post, we will discuss what code smells are and different categories of code smells.
Code smells are like the unpleasant odors that can linger in a room, even when it’s cleaned. They indicate underlying problems or design flaws in your code. Just as a bad smell might signal a leak or a forgotten piece of food, code smells can hint at issues like inefficient algorithms, redundant code, or poor naming conventions. Identifying and addressing these code smells is crucial for maintaining clean, maintainable, and efficient software.
What are Code Smells?
Code smells are indicators of potential problems or design flaws in software code. They often manifest as repetitive patterns, inconsistencies, or inefficient practices. These smells can range from simple issues like unused variables or long methods to more complex problems like tight coupling or god objects. While code smells may not always lead to immediate errors, they can make code harder to understand, maintain, and extend. Ignoring code smells can result in increased technical debt, reduced code quality, and potential bugs, ultimately hindering the development and evolution of software projects.
1. Bloaters
Bloaters are a type of code smell that refers to code elements that are excessively large or complex. They can significantly impact code readability, maintainability, and performance. This blog post will delve into what bloaters are, their negative consequences, common types, and strategies for addressing and preventing them.
What are Bloaters?
Bloaters are code elements that have grown beyond a reasonable size or complexity. They often make code difficult to understand, modify, and test. Common examples of bloaters include:
- Large Classes: Classes with an excessive number of responsibilities or methods.
- Long Methods: Methods that contain too many lines of code, making them difficult to follow and maintain.
- God Objects: Objects that handle too many responsibilities, making them tightly coupled and hard to modify.
- Primitive Obsession: Excessive use of primitive data types instead of creating custom data structures.
The Impact of Bloaters
Leaving bloaters untouched in your code can have several negative consequences:
- Reduced Readability: Bloaters make code harder to understand, as it becomes difficult to grasp the overall logic and intent.
- Increased Maintenance Costs: Modifying bloaters can be time-consuming and error-prone, leading to higher maintenance costs.
- Slower Development: Bloaters can hinder development productivity as developers struggle to work with complex and hard-to-understand code.
- Reduced Testability: Bloaters can make it difficult to write effective unit tests, as they often have many dependencies and complex logic.
Types of Bloaters
1. Large Classes
A large class violates Single Responsibility Principle (SRP) when:
- It’s hard to understand: A class that’s too complex or has too many responsibilities can be difficult to comprehend, leading to errors and misunderstandings.
- It’s difficult to maintain: Making changes to a large class can be risky and time-consuming, as it’s more likely to introduce bugs or break existing functionality.
- It’s hard to test: Large classes can be difficult to test effectively, as they often have many dependencies and complex logic.
In essence, the problem with large classes isn’t their size per se, but rather the negative consequences that can arise from their complexity. If a class is large but well-structured, modular, and easy to understand, it may not be a significant issue. However, if a large class is difficult to work with, it’s likely violating SRP and should be refactored.
Example:
// Before Refactoring
public class Employee {
private String name;
private String email;
private double salary;
private String department;
public Employee(String name, String email, double salary, String department) {
this.name = name;
this.email = email;
this.salary = salary;
this.department = department;
}
public void saveToDatabase() {
// database logic
}
public void sendWelcomeEmail() {
// email logic
}
public void calculateTax() {
// tax calculation logic
}
public void generatePaySlip() {
// pay slip generation logic
}
public void promoteEmployee() {
// promotion logic
}
// getters and setters
}This class handles multiple responsibilities, making it difficult to understand and modify.
Here is a better version :
// After Refactoring
public class Employee {
private String name;
private String email;
private double salary;
private String department;
public Employee(String name, String email, double salary, String department) {
this.name = name;
this.email = email;
this.salary = salary;
this.department = department;
}
// getters and setters
}
public class EmployeeDatabase {
public void saveToDatabase(Employee employee) {
// database logic
}
}
public class EmployeeEmailService {
public void sendWelcomeEmail(Employee employee) {
// email logic
}
}
public class TaxCalculator {
public double calculateTax(Employee employee) {
// tax calculation logic
}
}
public class PaySlipGenerator {
public void generatePaySlip(Employee employee) {
// pay slip generation logic
}
}
public class EmployeePromotionService {
public void promoteEmployee(Employee employee) {
// promotion logic
}
}2. Long Methods
Long method also violate the Single Responsibility Principle for similar reasons outlined above.
Example:
// Before Refactoring
public class OrderProcessor {
public void processOrder(Order order) {
// Calculate order total
double total = order.getSubtotal() + order.getTax() + order.getShipping();
// Update order status
order.setStatus("processed");
// Save order to database
Database.saveOrder(order);
// Send confirmation email
EmailService.sendEmail(order.getCustomer().getEmail(), "Order Confirmation",
"Thank you for your order!");
// Generate invoice
Invoice invoice = new Invoice(order);
invoice.generatePdf();
// Save invoice to file system
FileService.saveFile(invoice.getPdf(), "invoices/" + order.getId() + ".pdf");
// Update customer loyalty points
CustomerLoyaltyProgram.updatePoints(order.getCustomer(), order.getTotal());
}
}This method is too long and complex, making it hard to read and debug.
Here is a better version :
// After Refactoring
public class OrderProcessor {
public void processOrder(Order order) {
calculateOrderTotal(order);
updateOrderStatus(order);
saveOrderToDatabase(order);
sendConfirmationEmail(order);
generateAndSaveInvoice(order);
updateCustomerLoyaltyPoints(order);
}
private void calculateOrderTotal(Order order) {
double total = order.getSubtotal() + order.getTax() + order.getShipping();
order.setTotal(total);
}
private void updateOrderStatus(Order order) {
order.setStatus("processed");
}
private void saveOrderToDatabase(Order order) {
Database.saveOrder(order);
}
private void sendConfirmationEmail(Order order) {
EmailService.sendEmail(order.getCustomer().getEmail(), "Order Confirmation",
"Thank you for your order!");
}
private void generateAndSaveInvoice(Order order) {
Invoice invoice = new Invoice(order);
invoice.generatePdf();
FileService.saveFile(invoice.getPdf(), "invoices/" + order.getId() + ".pdf");
}
private void updateCustomerLoyaltyPoints(Order order) {
CustomerLoyaltyProgram.updatePoints(order.getCustomer(), order.getTotal());
}
}3. God Objects
God objects are an issue again because they violate the principle of single responsibility. When an object takes on too many responsibilities, it becomes difficult to understand, maintain, and test.
Example:
// Before Refactoring
public class ShoppingCart {
private List<Product> products;
private Customer customer;
private PaymentGateway paymentGateway;
private Inventory inventory;
private ShippingCalculator shippingCalculator;
public ShoppingCart(Customer customer) {
this.customer = customer;
}
public void addProduct(Product product) {
products.add(product);
inventory.decrementProductQuantity(product);
}
public void removeProduct(Product product) {
products.remove(product);
inventory.incrementProductQuantity(product);
}
public void checkout() {
double subtotal = calculateSubtotal();
double tax = calculateTax(subtotal);
double shippingCost = shippingCalculator.calculateShippingCost(customer.getAddress(), products);
paymentGateway.chargeCard(customer.getPaymentMethod(), subtotal + tax + shippingCost);
customer.setLoyaltyPoints(customer.getLoyaltyPoints() + 10);
inventory.updateStockLevels(products);
}
private double calculateSubtotal() {
return products.stream().mapToDouble(Product::getPrice).sum();
}
private double calculateTax(double subtotal) {
// tax calculation logic
}
}This object handles too many responsibilities, making it tightly coupled and difficult to modify.
Here is a better version :
// After Refactoring
public class ShoppingCart {
private List<Product> products;
private CheckoutService checkoutService;
public ShoppingCart(CheckoutService checkoutService) {
this.checkoutService = checkoutService;
}
public void addProduct(Product product) {
products.add(product);
}
public void removeProduct(Product product) {
products.remove(product);
}
public void checkout(Customer customer) {
checkoutService.processCheckout(customer, products);
}
}
public class CheckoutService {
private PaymentGateway paymentGateway;
private Inventory inventory;
private ShippingCalculator shippingCalculator;
private LoyaltyProgram loyaltyProgram;
public CheckoutService(PaymentGateway paymentGateway, Inventory inventory,
ShippingCalculator shippingCalculator, LoyaltyProgram loyaltyProgram) {
this.paymentGateway = paymentGateway;
this.inventory = inventory;
this.shippingCalculator = shippingCalculator;
this.loyaltyProgram = loyaltyProgram;
}
public void processCheckout(Customer customer, List<Product> products) {
double subtotal = calculateSubtotal(products);
double tax = calculateTax(subtotal);
double shippingCost = shippingCalculator.calculateShippingCost(customer.getAddress(), products);
paymentGateway.chargeCard(customer.getPaymentMethod(), subtotal + tax + shippingCost);
loyaltyProgram.updatePoints(customer, 10);
inventory.updateStockLevels(products);
}
private double calculateSubtotal(List<Product> products) {
return products.stream().mapToDouble(Product::getPrice).sum();
}
private double calculateTax(double subtotal) {
// tax calculation logic
}
}4. Primitive Obsession
Primitive obsession is a code smell that occurs when a class or method relies excessively on primitive data types (like int, float, boolean, String, etc.) instead of creating custom data structures.
Here’s why primitive obsession is an issue:
- Lack of encapsulation: By using primitive types directly, you’re not encapsulating related data and behavior within a cohesive object. This can make your code harder to understand, maintain, and reuse.
- Increased coupling: Using primitive types can lead to tighter coupling between different parts of your code, making it more difficult to make changes without affecting other areas.
- Reduced flexibility: Primitive types are less flexible than custom data structures, as they don’t provide the same level of abstraction or encapsulation. This can limit your ability to reuse code and make changes in the future.
- Increased complexity: Overusing primitive types can make your code more complex and harder to understand, especially when dealing with complex data relationships.
Example:
// Before Refactoring
public class Employee {
private String name;
private int age;
private String addressLine1;
private String addressLine2;
private String city;
private String state;
private String zipCode;
private boolean isAdmin;
public Employee(String name, int age, String addressLine1, String addressLine2,
String city, String state, String zipCode, boolean isAdmin) {
this.name = name;
this.age = age;
this.addressLine1 = addressLine1;
this.addressLine2 = addressLine2;
this.city = city;
this.state = state;
this.zipCode = zipCode;
this.isAdmin = isAdmin;
}
public String getAddress() {
return addressLine1 + ", " + addressLine2 + ", " + city + ", " + state + " " + zipCode;
}
public boolean isAdult() {
return age >= 18;
}
public boolean isEligibleForAdministrativeRole() {
return isAdmin && isAdult();
}
}Instead of using a custom Address class, this code relies on primitive strings, making it less reusable and harder to maintain.
Here is a better one :
// After Refactoring
public class Employee {
private String name;
private Age age;
private Address address;
private Role role;
public Employee(String name, Age age, Address address, Role role) {
this.name = name;
this.age = age;
this.address = address;
this.role = role;
}
public String getAddress() {
return address.toString();
}
public boolean isEligibleForAdministrativeRole() {
return role.isAdmin() && age.isAdult();
}
}
public class Address {
private String line1;
private String line2;
private City city;
private State state;
private ZipCode zipCode;
public Address(String line1, String line2, City city, State state, ZipCode zipCode) {
this.line1 = line1;
this.line2 = line2;
this.city = city;
this.state = state;
this.zipCode = zipCode;
}
@Override
public String toString() {
return line1 + ", " + line2 + ", " + city + ", " + state + " " + zipCode;
}
}
public class Age {
private int value;
public Age(int value) {
this.value = value;
}
public boolean isAdult() {
return value >= 18;
}
}
public enum Role {
ADMIN(true),
NON_ADMIN(false);
private boolean isAdmin;
Role(boolean isAdmin) {
this.isAdmin = isAdmin;
}
public boolean isAdmin() {
return isAdmin;
}
}Fixing Bloaters
To address bloaters, you need to refactor your code into smaller, more focused elements. Here are some key strategies:
- Extract Methods: Break down long methods into smaller, more focused methods.
- Extract Classes: Identify responsibilities that can be encapsulated in separate classes.
- Introduce Data Structures: Create custom data structures to represent related data.
- Use Design Patterns: Consider using design patterns like Strategy, Observer, or Facade to improve code structure.
Important Considerations:
- Test-Driven Development: Write unit tests before and after refactoring to ensure that your changes don’t introduce new bugs.
- Incremental Changes: Refactor your code gradually to avoid introducing errors or breaking existing functionality.
- Code Review: Have your code reviewed by others to get feedback and ensure that your changes are well-structured and maintainable.
Preventing Bloaters
To prevent bloaters from occurring in the first place, follow these guidelines:
- Plan Ahead: Design your code carefully from the beginning to avoid introducing unnecessary complexity.
- Write Clean Code: Adhere to coding conventions and best practices to improve code readability and maintainability.
- Refactor Regularly: Continuously refactor your code to keep it clean and well-structured.
- Code Review: Have your code reviewed by others to identify potential bloaters and areas for improvement.
2. Object-Oriented Abusers
Object-Oriented Abusers (OOAs) are code smells that occur when object-oriented principles are misused or misunderstood. They can significantly impact code readability, maintainability, and performance. This blog post will delve into what OOAs are, their negative consequences, common types, and strategies for addressing and preventing them.
What are Object-Oriented Abusers?
OOAs are code elements that violate or misuse object-oriented principles, such as encapsulation, inheritance, polymorphism, and abstraction. They often make code harder to understand, modify, and test. Common examples of OOAs include:
- God Objects: Objects that handle too many responsibilities, making them tightly coupled and difficult to modify.
- Primitive Obsession: Excessive use of primitive data types instead of creating custom data structures.
- Switch Statements: Overuse of switch statements to determine behavior based on object types, which can lead to tight coupling and lack of polymorphism.
- Lazy Classes: Classes that have little or no behavior, often containing only data members.
- Refused Bequest: Inheritance hierarchies that are too deep or where subclasses inherit unnecessary methods or attributes.
The Impact of Object-Oriented Abusers
Leaving OOAs untouched in your code can have several negative consequences:
- Reduced Readability: OOAs can make code harder to understand, as they violate expected object-oriented patterns and principles.
- Increased Maintenance Costs: Modifying code with OOAs can be time-consuming and error-prone, leading to higher maintenance costs.
- Slower Development: OOAs can hinder development productivity as developers struggle to work with complex and hard-to-understand code.
- Reduced Testability: OOAs can make it difficult to write effective unit tests, as they often have many dependencies and complex logic.
Types of Object-Oriented Abusers
1. God Objects
This object handles too many responsibilities, making it tightly coupled and difficult to modify. This is also a bloater and refer to bloaters for a detailed explanation.
2. Primitive Obsession
Already discussed as a bloater.
3. Switch Statements
Example:
// Before Refactoring
public class PaymentProcessor {
public void processPayment(Payment payment) {
switch (payment.getType()) {
case CREDIT_CARD:
processCreditCardPayment(payment);
break;
case PAYPAL:
processPayPalPayment(payment);
break;
case BANK_TRANSFER:
processBankTransferPayment(payment);
break;
default:
throw new UnsupportedOperationException("Unsupported payment type");
}
}
private void processCreditCardPayment(Payment payment) {
// credit card payment logic
}
private void processPayPalPayment(Payment payment) {
// paypal payment logic
}
private void processBankTransferPayment(Payment payment) {
// bank transfer payment logic
}
}
public enum PaymentType {
CREDIT_CARD,
PAYPAL,
BANK_TRANSFER
}
public class Payment {
private PaymentType type;
public Payment(PaymentType type) {
this.type = type;
}
public PaymentType getType() {
return type;
}
}In this example:
- The
PaymentProcessorclass uses a switch statement to determine payment processing logic - Addition of new payment types requires modifying the switch statement
Here is a better one :
// After Refactoring
public interface PaymentProcessorStrategy {
void processPayment(Payment payment);
}
public class CreditCardPaymentProcessor implements PaymentProcessorStrategy {
@Override
public void processPayment(Payment payment) {
// credit card payment logic
}
}
public class PayPalPaymentProcessor implements PaymentProcessorStrategy {
@Override
public void processPayment(Payment payment) {
// paypal payment logic
}
}
public class BankTransferPaymentProcessor implements PaymentProcessorStrategy {
@Override
public void processPayment(Payment payment) {
// bank transfer payment logic
}
}
public class PaymentProcessor {
private Map<PaymentType, PaymentProcessorStrategy> strategies;
public PaymentProcessor() {
strategies = new HashMap<>();
strategies.put(PaymentType.CREDIT_CARD, new CreditCardPaymentProcessor());
strategies.put(PaymentType.PAYPAL, new PayPalPaymentProcessor());
strategies.put(PaymentType.BANK_TRANSFER, new BankTransferPaymentProcessor());
}
public void processPayment(Payment payment) {
PaymentProcessorStrategy strategy = strategies.get(payment.getType());
if (strategy != null) {
strategy.processPayment(payment);
} else {
throw new UnsupportedOperationException("Unsupported payment type");
}
}
}4. Lazy Classes
A Lazy Class is a code smell that occurs when a class doesn’t pull its weight, often because it:
- Has too few methods or responsibilities
- Doesn’t encapsulate data or behavior
- Is only used in one place
Example:
// Before Refactoring
public class User {
private String name;
private String email;
public User(String name, String email) {
this.name = name;
this.email = email;
}
public String getName() {
return name;
}
public String getEmail() {
return email;
}
}
public class UserService {
public void createUser(String name, String email) {
User user = new User(name, email);
// create user logic
System.out.println("User created: " + user.getName() + " (" + user.getEmail() + ")");
}
public void updateUser(String name, String email) {
User user = new User(name, email);
// update user logic
System.out.println("User updated: " + user.getName() + " (" + user.getEmail() + ")");
}
public void deleteUser(String name, String email) {
User user = new User(name, email);
// delete user logic
System.out.println("User deleted: " + user.getName() + " (" + user.getEmail() + ")");
}
}In this example:
- The
Userclass only has getters and no behavior - The
UserServiceclass createsUserobjects but doesn't reuse them
Issues:
Userclass doesn't encapsulate behaviorUserServiceclass duplicates user creation logic- Tight coupling between
UserServiceandUser
Here is a better one :
// After Refactoring
public class User {
private String name;
private String email;
public User(String name, String email) {
this.name = name;
this.email = email;
}
public String getName() {
return name;
}
public String getEmail() {
return email;
}
public void save() {
// save user logic
System.out.println("User saved: " + name + " (" + email + ")");
}
public void update() {
// update user logic
System.out.println("User updated: " + name + " (" + email + ")");
}
public void delete() {
// delete user logic
System.out.println("User deleted: " + name + " (" + email + ")");
}
}
public class UserService {
public void manageUser(User user, String action) {
switch (action) {
case "create":
user.save();
break;
case "update":
user.update();
break;
case "delete":
user.delete();
break;
}
}
}5. Refused Bequest
A Refused Bequest occurs when a subclass inherits behavior or properties from its superclass that it doesn’t need or want.
Example:
// Before Refactoring
public abstract class Vehicle {
private String color;
private int wheels;
public Vehicle(String color, int wheels) {
this.color = color;
this.wheels = wheels;
}
public String getColor() {
return color;
}
public int getWheels() {
return wheels;
}
public void honkHorn() {
System.out.println("Beep beep!");
}
public void accelerate() {
System.out.println("Accelerating...");
}
}
public class Car extends Vehicle {
public Car(String color) {
super(color, 4);
}
public void openTrunk() {
System.out.println("Trunk opened");
}
}
public class Bicycle extends Vehicle {
public Bicycle(String color) {
super(color, 2);
}
public void ringBell() {
System.out.println("Ring ring!");
}
}Here is a better version of the code
// After Refactoring
public interface Vehicle {
String getColor();
}
public interface Hornable {
void honkHorn();
}
public interface Accelerable {
void accelerate();
}
public abstract class WheeledVehicle implements Vehicle {
private int wheels;
public WheeledVehicle(int wheels) {
this.wheels = wheels;
}
public int getWheels() {
return wheels;
}
}
public class Car extends WheeledVehicle implements Hornable, Accelerable {
public Car(String color) {
super(4);
}
@Override
public String getColor() {
return "Default color";
}
@Override
public void honkHorn() {
System.out.println("Beep beep!");
}
@Override
public void accelerate() {
System.out.println("Accelerating...");
}
public void openTrunk() {
System.out.println("Trunk opened");
}
}
public class Bicycle extends WheeledVehicle {
public Bicycle(String color) {
super(2);
}
@Override
public String getColor() {
return "Default color";
}
public void ringBell() {
System.out.println("Ring ring!");
}
}Fixing Object-Oriented Abusers
To address OOAs, you need to refactor your code to better adhere to object-oriented principles. Here are some key strategies:
- Break Down God Objects: Identify responsibilities that can be encapsulated in separate classes.
- Introduce Data Structures: Create custom data structures to represent related data.
- Use Polymorphism: Use polymorphism to avoid switch statements and make your code more flexible.
- Add Behavior to Lazy Classes: Give lazy classes meaningful responsibilities to make them more useful.
- Refactor Inheritance Hierarchies: Simplify inheritance hierarchies and avoid unnecessary inheritance.
Important Considerations:
- Test-Driven Development: Write unit tests before and after refactoring to ensure that your changes don’t introduce new bugs.
- Incremental Changes: Refactor your code gradually to avoid introducing errors or breaking existing functionality.
- Code Review: Have your code reviewed by others to get feedback and ensure that your changes are well-structured and maintainable.
Preventing Object-Oriented Abusers
To prevent OOAs from occurring in the first place, follow these guidelines:
- Plan Ahead: Design your code carefully from the beginning to avoid introducing unnecessary complexity.
- Write Clean Code: Adhere to coding conventions and best practices to improve code readability and maintainability.
- Refactor Regularly: Continuously refactor your code to keep it clean and well-structured.
- Code Review: Have your code reviewed by others to identify potential OOAs and areas for improvement.
3. Change Preventers
Change Preventers are a type of code smell that make it difficult to modify code without introducing unintended side effects. They can significantly impact code maintainability, flexibility, and testability. This blog post will delve into what Change Preventers are, their negative consequences, common types, and strategies for addressing and preventing them.
What are Change Preventers?
Change Preventers are code elements that make it difficult to change code in one place without making many changes elsewhere. They often introduce tight coupling and dependencies that make the codebase brittle and hard to maintain. Common examples of Change Preventers include:
- Tight Coupling: When classes or components are highly dependent on each other, making it difficult to modify one without affecting the other.
- Fragile Tests: Tests that are overly sensitive to small changes in the code, making them difficult to maintain and unreliable.
- Divergent Changes: When different parts of the codebase are evolving independently, making it difficult to merge changes and maintain consistency.
The Impact of Change Preventers
Leaving Change Preventers untouched in your code can have several negative consequences:
- Reduced Maintainability: Code with Change Preventers is difficult to modify without introducing unintended side effects, making it harder to maintain and evolve.
- Increased Risk of Bugs: Tight coupling and fragile tests can increase the risk of introducing bugs when making changes.
- Slower Development: Change Preventers can hinder development productivity as developers struggle to make changes without causing unintended consequences.
- Reduced Flexibility: Code with Change Preventers is less flexible and adaptable to changing requirements, making it more difficult to introduce new features or modify existing ones.
Types of Change Preventers
1. Tight Coupling
Example:
// Before Refactoring
public class PaymentGateway {
public void processPayment(Payment payment) {
// payment processing logic
System.out.println("Payment processed through PayPal");
}
}
public class PayPalPaymentGateway extends PaymentGateway {
public PayPalPaymentGateway() {
super();
}
}
public class StripePaymentGateway extends PaymentGateway {
public StripePaymentGateway() {
super();
}
}
public class PaymentProcessor {
private PaymentGateway paymentGateway;
public PaymentProcessor() {
this.paymentGateway = new PayPalPaymentGateway();
}
public void processPayment(Payment payment) {
paymentGateway.processPayment(payment);
}
}In this example:
PaymentProcessoris tightly coupled toPayPalPaymentGateway.- Changing the payment gateway requires modifying
PaymentProcessor.
Here is a better version :
// After Refactoring
public interface PaymentGateway {
void processPayment(Payment payment);
}
public class PayPalPaymentGateway implements PaymentGateway {
@Override
public void processPayment(Payment payment) {
// payment processing logic
System.out.println("Payment processed through PayPal");
}
}
public class StripePaymentGateway implements PaymentGateway {
@Override
public void processPayment(Payment payment) {
// payment processing logic
System.out.println("Payment processed through Stripe");
}
}
public class PaymentProcessor {
private PaymentGateway paymentGateway;
public PaymentProcessor(PaymentGateway paymentGateway) {
this.paymentGateway = paymentGateway;
}
public void processPayment(Payment payment) {
paymentGateway.processPayment(payment);
}
}2. Fragile Tests
Example:
// Before Refactoring
public class UserService {
public User createUser(String name, String email) {
// create user logic
return new User(name, email);
}
}
public class User {
private String name;
private String email;
public User(String name, String email) {
this.name = name;
this.email = email;
}
public String getName() {
return name;
}
public String getEmail() {
return email;
}
}
// Fragile Test
@Test
public void testCreateUser() {
UserService userService = new UserService();
User user = userService.createUser("John Doe", "john.doe@example.com");
assertEquals("John Doe", user.getName());
assertEquals("john.doe@example.com", user.getEmail());
assertEquals(2, user.getName().split("\\s+").length); // Fragile assertion
}In this example:
- The test
testCreateUser()is fragile because it asserts the exact format of the user's name. - Changing the name format (e.g., adding a middle name) would break the test.
Here is a better version :
// After Refactoring
@Test
public void testCreateUser() {
UserService userService = new UserService();
User user = userService.createUser("John Doe", "john.doe@example.com");
assertEquals("John Doe", user.getName());
assertEquals("john.doe@example.com", user.getEmail());
assertTrue(user.getName().contains("John")); // More robust assertion
assertTrue(user.getName().contains("Doe"));
}3. Divergent Changes
Example:
// Before Refactoring
public class OrderService {
public void placeOrder(Order order) {
// Place order logic
System.out.println("Order placed");
}
public void cancelOrder(Order order) {
// Cancel order logic
System.out.println("Order cancelled");
}
public void updateOrderStatus(Order order, String status) {
// Update order status logic
System.out.println("Order status updated to " + status);
}
public void calculateOrderTotal(Order order) {
// Calculate order total logic
System.out.println("Order total: $100");
}
public void generateInvoice(Order order) {
// Generate invoice logic
System.out.println("Invoice generated");
}
}Consider a codebase where one module is being refactored to use a new design pattern, while another module is being updated to support a new feature. If these changes are not carefully coordinated, they can lead to conflicts and inconsistencies.
Here is a better version :
// After Refactoring
public interface OrderService {
void placeOrder(Order order);
}
public class OrderPlacementService implements OrderService {
@Override
public void placeOrder(Order order) {
// Place order logic
System.out.println("Order placed");
}
}
public class OrderCancellationService {
public void cancelOrder(Order order) {
// Cancel order logic
System.out.println("Order cancelled");
}
}
public class OrderStatusService {
public void updateOrderStatus(Order order, String status) {
// Update order status logic
System.out.println("Order status updated to " + status);
}
}
public class OrderCalculator {
public void calculateOrderTotal(Order order) {
// Calculate order total logic
System.out.println("Order total: $100");
}
}
public class InvoiceGenerator {
public void generateInvoice(Order order) {
// Generate invoice logic
System.out.println("Invoice generated");
}
}Fixing Change Preventers
To address Change Preventers, you need to refactor your code to reduce coupling and improve modularity. Here are some key strategies:
- Dependency Injection: Use dependency injection to decouple components and make them more flexible.
- Extract Interfaces: Create interfaces to define the contracts between components, making them more loosely coupled.
- Refactor Tests: Make tests more robust and less sensitive to small changes in the code.
- Coordinate Changes: Carefully coordinate changes to different parts of the codebase to avoid conflicts and inconsistencies.
Important Considerations:
- Test-Driven Development: Write unit tests before and after refactoring to ensure that your changes don’t introduce new bugs.
- Incremental Changes: Refactor your code gradually to avoid introducing errors or breaking existing functionality.
- Code Review: Have your code reviewed by others to get feedback and ensure that your changes are well-structured and maintainable.
Preventing Change Preventers
To prevent Change Preventers from occurring in the first place, follow these guidelines:
- Design for Change: Design your code with change in mind, considering how different parts of the codebase might evolve over time.
- Write Clean Code: Adhere to coding conventions and best practices to improve code readability and maintainability.
- Refactor Regularly: Continuously refactor your code to keep it clean and well-structured.
- Code Review: Have your code reviewed by others to identify potential Change Preventers and areas for improvement.
4. Dispensable
Dispensable is a code smell that refers to code elements that are unnecessary or redundant. They can clutter your codebase, making it harder to understand, maintain, and test. This blog post will delve into what Dispensables are, their negative consequences, common types, and strategies for addressing and preventing them.
What are Dispensable?
Dispensable code elements are those that can be removed without affecting the functionality of the application. They often include:
- Unused Code: Methods, variables, or classes that are never used.
- Duplicate Code: Code that is repeated in multiple places.
- Unnecessary Comments: Comments that don’t add value or are outdated.
- Magic Numbers: Hardcoded numerical values that are not defined as constants.
The Impact of Dispensable
Leaving Dispensable code untouched in your codebase can have several negative consequences:
- Reduced Readability: Dispensable code can clutter your codebase and make it harder to understand.
- Increased Maintenance Costs: Maintaining Dispensable code can be time-consuming and error-prone.
- Slower Development: Dispensable code can hinder development productivity as developers navigate through unnecessary code.
- Increased Risk of Bugs: Dispensable code can introduce potential bugs or inconsistencies.
Types of Dispensable
1. Unused Code
Example:
// Before Refactoring
public class User {
private String name;
private String email;
public User(String name, String email) {
this.name = name;
this.email = email;
}
public String getName() {
return name;
}
public String getEmail() {
return email;
}
public void setAddress(String address) { // Unused method
// This method is never called
}
private String getUnusedVariable() { // Unused method
return "Unused variable";
}
}
public class UserService {
public void createUser(String name, String email) {
User user = new User(name, email);
// Create user logic
}
}In this example, unusedVariable and unusedMethod are Dispensable because they are not used anywhere in the code.
Here is a better version of the code :
// After Refactoring
public class User {
private String name;
private String email;
public User(String name, String email) {
this.name = name;
this.email = email;
}
public String getName() {
return name;
}
public String getEmail() {
return email;
}
}
public class UserService {
public void createUser(String name, String email) {
User user = new User(name, email);
// Create user logic
}
}2. Duplicate Code
Example:
Java
public class StringUtils {
public static String reverseString(String str) {
// ... code to reverse the string
}
}
public class AnotherClass {
public String reverseString(String str) {
// ... code to reverse the string (identical to StringUtils)
}
}The reverseString methods in both classes are duplicate code.
3. Unnecessary Comments
Example:
Java
// This method calculates the total
public int calculateTotal(List<Item> items) {
// ... code to calculate the total
}The comment in this example is unnecessary as the method name clearly indicates its purpose.
4. Magic Numbers
Example:
Java
public void processOrder(Order order) {
if (order.getStatus() == 1) {
// ... process order
}
}The hardcoded value 1 is a Magic Number and should be defined as a constant.
Fixing Dispensable Code
To address Dispensable code, you need to remove or refactor it. Here are some key strategies:
- Delete Unused Code: Remove unused methods, variables, or classes.
- Consolidate Duplicate Code: Create a shared method or function to avoid code duplication.
- Remove Unnecessary Comments: Delete comments that don’t add value or are outdated.
- Introduce Constants: Define constants for Magic Numbers to improve code readability and maintainability.
Important Considerations:
- Test-Driven Development: Write unit tests before and after refactoring to ensure that your changes don’t introduce new bugs.
- Incremental Changes: Refactor your code gradually to avoid introducing errors or breaking existing functionality.
- Code Review: Have your code reviewed by others to get feedback and ensure that your changes are well-structured and maintainable.
Preventing Dispensable Code
To prevent Dispensable code from occurring in the first place, follow these guidelines:
- Write Clean Code: Adhere to coding conventions and best practices to improve code readability and maintainability.
- Refactor Regularly: Continuously refactor your code to keep it clean and well-structured.
- Code Review: Have your code reviewed by others to identify potential Dispensable code and areas for improvement.
- Use Static Analysis Tools: Use static analysis tools to automatically detect Dispensable code.
5. Couplers
Couplers are a type of code smell that indicates a strong dependency between components or modules. They can make your code difficult to understand, maintain, and test. This blog post will delve into what Couplers are, their negative consequences, common types, and strategies for addressing and preventing them.
What are Couplers?
Couplers are code elements that create a tight coupling between different parts of your application. This can make it difficult to modify or reuse components independently. Common examples of Couplers include:
- Tight Coupling: When classes or components are highly dependent on each other, making it difficult to modify one without affecting the other.
- Feature Envy: When a method or function accesses data or performs operations that belong to another class.
- Message Chains: When a series of method calls are chained together, making it difficult to understand and maintain.
- Primitive Obsession: Excessive use of primitive data types instead of creating custom data structures.
The Impact of Couplers
Leaving Couplers untouched in your code can have several negative consequences:
- Reduced Readability: Couplers can make your code harder to understand, as it becomes difficult to follow the flow of data and control.
- Increased Maintenance Costs: Modifying code with Couplers can be time-consuming and error-prone, leading to higher maintenance costs.
- Slower Development: Couplers can hinder development productivity as developers struggle to work with complex and hard-to-understand code.
- Reduced Testability: Couplers can make it difficult to write effective unit tests, as they often have many dependencies and complex logic.
Types of Couplers
1. Tight Coupling
We already discussed about Tight coupling earlier. So we are not detailing about this here.
2. Feature Envy
Feature Envy occurs when a method or class uses or modifies the data of another class more than its own.
Example:
// Before Refactoring
public class User {
private String name;
private String email;
public User(String name, String email) {
this.name = name;
this.email = email;
}
public String getName() {
return name;
}
public String getEmail() {
return email;
}
}
public class UserService {
public void createUser(String name, String email) {
User user = new User(name, email);
user.setName(user.getName().trim()); // Modifying User's data
user.setEmail(user.getEmail().toLowerCase()); // Modifying User's data
// Create user logic
}
public boolean isValidEmail(String email) {
return email.contains("@"); // Using User's data
}
public String getUserName(String name) {
return name.substring(0, 1).toUpperCase() + name.substring(1); // Using User's data
}
}In this example:
- The
UserServiceclass modifies and uses theUserclass's data more than its own. - The
UserServiceclass has intimate knowledge of theUserclass's internal state.
Here is a better version of the code :
// After Refactoring
public class User {
private String name;
private String email;
public User(String name, String email) {
this.name = name.trim();
this.email = email.toLowerCase();
}
public String getName() {
return name;
}
public String getEmail() {
return email;
}
public boolean hasValidEmail() {
return email.contains("@");
}
public String getFormattedName() {
return name.substring(0, 1).toUpperCase() + name.substring(1);
}
}
public class UserService {
public void createUser(User user) {
// Create user logic
}
}3. Message Chains
Message Chains occur when a client requests an object from another object, which in turn requests another object, creating a chain of requests.
Example:
// Before Refactoring
public class University {
private Department department;
public University(Department department) {
this.department = department;
}
public Department getDepartment() {
return department;
}
}
public class Department {
private Professor professor;
public Department(Professor professor) {
this.professor = professor;
}
public Professor getProfessor() {
return professor;
}
}
public class Professor {
private Course course;
public Professor(Course course) {
this.course = course;
}
public Course getCourse() {
return course;
}
}
public class Course {
private String name;
public Course(String name) {
this.name = name;
}
public String getName() {
return name;
}
}
public class Client {
public void printCourseName(University university) {
String courseName = university.getDepartment().getProfessor().getCourse().getName();
System.out.println(courseName);
}
}This code chain is difficult to read and understand, as it involves multiple method calls.
Better version :
// After Refactoring
public class University {
public String getCourseName() {
return department.getCourseName();
}
private Department department;
}
public class Department {
public String getCourseName() {
return professor.getCourseName();
}
private Professor professor;
}
public class Professor {
public String getCourseName() {
return course.getName();
}
private Course course;
}
public class Course {
public String getName() {
return name;
}
private String name;
}
public class Client {
public void printCourseName(University university) {
String courseName = university.getCourseName();
System.out.println(courseName);
}
}4. Primitive Obsession
We already discussed this code smell above.
Fixing Couplers
To address Couplers, you need to refactor your code to reduce coupling and improve modularity. Here are some key strategies:
- Dependency Injection: Use dependency injection to decouple components and make them more flexible.
- Extract Interfaces: Create interfaces to define the contracts between components, making them more loosely coupled.
- Refactor Methods: Break down methods that exhibit Feature Envy into smaller, more focused methods.
- Introduce Data Structures: Create custom data structures to represent related data, reducing the need for message chains.
- Avoid Primitive Obsession: Use custom data structures instead of primitive types whenever possible.
Important Considerations:
- Test-Driven Development: Write unit tests before and after refactoring to ensure that your changes don’t introduce new bugs.
- Incremental Changes: Refactor your code gradually to avoid introducing errors or breaking existing functionality.
- Code Review: Have your code reviewed by others to get feedback and ensure that your changes are well-structured and maintainable.
Preventing Couplers
To prevent Couplers from occurring in the first place, follow these guidelines:
- Design for Change: Design your code with change in mind, considering how different parts of the codebase might evolve over time.
- Write Clean Code: Adhere to coding conventions and best practices to improve code readability and maintainability.
- Refactor Regularly: Continuously refactor your code to keep it clean and well-structured.
- Code Review: Have your code reviewed by others to identify potential Couplers and areas for improvement.