Skip to main content

Code Smells and Refactoring

Code Smells

Like warning signs in a building that suggest potential structural issues. They don't necessarily mean the code is broken, but they hint that something could be improved to prevent future problems.

Understanding Code Smells

Code smells are surface indications that usually correspond to deeper problems in the code. They are not bugs—the code typically works correctly—but they suggest weaknesses in design that may slow down development or increase the risk of bugs in the future.

The term "code smell" was popularized by Kent Beck and Martin Fowler in their book "Refactoring: Improving the Design of Existing Code." Identifying code smells is a skill that improves with experience, and addressing them through refactoring is a key practice for maintaining healthy codebases.

Common Code Smells

Bloaters

Bloaters are code, methods, or classes that have grown too large over time:

Bloater Code Smells

📜

Long Method

A method that has grown too large, often doing too many things

Example: A 200-line checkout method that handles validation, payment, inventory, and email confirmation
🐘

Large Class

A class that has too many fields, methods, or responsibilities

Example: A UserManager class with 30 methods and 20 properties handling everything user-related
🔢

Primitive Obsession

Using primitive types instead of small objects for simple tasks

Example: Using strings for phone numbers, ZIP codes, or email addresses instead of specific types
📋

Long Parameter List

Methods with too many parameters that are hard to understand and use

Example: A createUser function with 10+ separate parameters instead of a user object

Long Method Refactoring

Breaking up a long method into smaller, focused methods

Long, Complex MethodAvoid
// Long, complex method with multiple responsibilities
function processOrder(order) {
// Input validation
if (!order.items || order.items.length === 0) {
throw new Error('Order must have at least one item');
}
if (!order.shippingAddress) {
throw new Error('Shipping address is required');
}
if (!order.paymentDetails) {
throw new Error('Payment details are required');
}

// Calculate totals
let subtotal = 0;
for (const item of order.items) {
if (!item.product || !item.quantity) {
throw new Error('Each item must have a product and quantity');
}

// Get product details
const product = database.getProduct(item.product.id);
if (!product) {
throw new Error(`Product not found: ${item.product.id}`);
}

// Check inventory
const inventoryItem = database.getInventoryItem(product.id);
if (!inventoryItem || inventoryItem.stock < item.quantity) {
throw new Error(`Insufficient stock for product: ${product.name}`);
}

// Update inventory
database.updateInventory(product.id, inventoryItem.stock - item.quantity);

// Calculate item price
const itemPrice = product.price * item.quantity;
subtotal += itemPrice;
}

// Apply tax
const taxRate = 0.08; // 8% tax
const tax = subtotal * taxRate;

// Calculate shipping
let shippingCost = 5.99; // Base shipping
if (subtotal > 50) {
shippingCost = 0; // Free shipping for orders over $50
}

// Calculate total
const total = subtotal + tax + shippingCost;

// Process payment
const paymentResult = paymentGateway.charge({
amount: total,
creditCard: order.paymentDetails.creditCard,
expiryDate: order.paymentDetails.expiryDate,
cvv: order.paymentDetails.cvv
});

if (!paymentResult.success) {
throw new Error(`Payment failed: ${paymentResult.error}`);
}

// Create order record
const orderRecord = database.createOrder({
items: order.items,
subtotal,
tax,
shipping: shippingCost,
total,
shippingAddress: order.shippingAddress,
paymentId: paymentResult.transactionId
});

// Send confirmation email
emailService.sendOrderConfirmation(
order.customerEmail,
{
orderId: orderRecord.id,
items: order.items.map(item => ({
product: database.getProduct(item.product.id).name,
quantity: item.quantity,
price: database.getProduct(item.product.id).price
})),
total: total
}
);

// Return order details
return {
orderId: orderRecord.id,
subtotal,
tax,
shipping: shippingCost,
total,
paymentId: paymentResult.transactionId
};
}
Refactored with Helper MethodsRecommended
// Main orchestration method with clear separation of concerns
function processOrder(order) {
validateOrder(order);

const orderItems = prepareOrderItems(order.items);
updateInventory(orderItems);

const { subtotal, tax, shippingCost, total } = calculateOrderAmounts(orderItems);
const paymentResult = processPayment(order.paymentDetails, total);

const orderRecord = createOrderRecord(order, orderItems, subtotal, tax, shippingCost, total, paymentResult);
sendOrderConfirmation(order.customerEmail, orderRecord, orderItems);

return buildOrderResult(orderRecord, subtotal, tax, shippingCost, total, paymentResult);
}

// Validation function
function validateOrder(order) {
if (!order.items || order.items.length === 0) {
throw new Error('Order must have at least one item');
}
if (!order.shippingAddress) {
throw new Error('Shipping address is required');
}
if (!order.paymentDetails) {
throw new Error('Payment details are required');
}
}

// Prepare and validate order items
function prepareOrderItems(items) {
return items.map(item => {
if (!item.product || !item.quantity) {
throw new Error('Each item must have a product and quantity');
}

const product = database.getProduct(item.product.id);
if (!product) {
throw new Error(`Product not found: ${item.product.id}`);
}

return {
item,
product,
itemPrice: product.price * item.quantity
};
});
}

// Update inventory levels
function updateInventory(orderItems) {
for (const { item, product } of orderItems) {
const inventoryItem = database.getInventoryItem(product.id);

if (!inventoryItem || inventoryItem.stock < item.quantity) {
throw new Error(`Insufficient stock for product: ${product.name}`);
}

database.updateInventory(product.id, inventoryItem.stock - item.quantity);
}
}

// Calculate financial amounts
function calculateOrderAmounts(orderItems) {
const subtotal = orderItems.reduce((sum, { itemPrice }) => sum + itemPrice, 0);
const taxRate = 0.08;
const tax = subtotal * taxRate;
const shippingCost = (subtotal > 50) ? 0 : 5.99;
const total = subtotal + tax + shippingCost;

return { subtotal, tax, shippingCost, total };
}

// Additional helper functions...
function processPayment(paymentDetails, amount) { /* ... */ }
function createOrderRecord(order, orderItems, subtotal, tax, shipping, total, paymentResult) { /* ... */ }
function sendOrderConfirmation(email, orderRecord, orderItems) { /* ... */ }
function buildOrderResult(orderRecord, subtotal, tax, shipping, total, paymentResult) { /* ... */ }

Object-Orientation Abusers

These smells indicate incomplete or incorrect application of object-oriented principles:

Object-Orientation Abuser Smells

🔀

Switch Statements

Complex conditional logic that should be replaced with polymorphism

Example: A long switch/case block that performs different actions based on an object type
🚫

Refused Bequest

A subclass uses only some of the methods and properties inherited from its parents

Example: A Square class that inherits from Rectangle but throws errors when setting width different from height
⏱️

Temporary Field

An object attribute that is set only in certain circumstances

Example: A User class with an 'activationToken' field that's only used during account creation
🔄

Alternative Classes with Different Interfaces

Two classes that do similar things but have different interfaces

Example: Both EmailNotifier and SMSAlertService doing essentially the same job with different method names

Switch Statement Refactoring

Replacing conditional logic with polymorphism

Switch-Based LogicAvoid
// Using switch statements for different behaviors
class PaymentProcessor {
processPayment(payment) {
switch(payment.type) {
case 'credit_card':
return this.processCreditCardPayment(payment);
case 'paypal':
return this.processPayPalPayment(payment);
case 'bank_transfer':
return this.processBankTransferPayment(payment);
case 'crypto':
return this.processCryptoPayment(payment);
default:
throw new Error(`Unsupported payment type: ${payment.type}`);
}
}

processCreditCardPayment(payment) {
console.log('Processing credit card payment');
// Validate credit card details
if (!payment.cardNumber || !payment.expiryDate || !payment.cvv) {
throw new Error('Invalid credit card details');
}
// Process with credit card gateway
return {
success: true,
transactionId: 'cc_' + Date.now()
};
}

processPayPalPayment(payment) {
console.log('Processing PayPal payment');
// Validate PayPal details
if (!payment.email) {
throw new Error('Invalid PayPal email');
}
// Process with PayPal API
return {
success: true,
transactionId: 'pp_' + Date.now()
};
}

processBankTransferPayment(payment) {
console.log('Processing bank transfer');
// Validate bank details
if (!payment.accountNumber || !payment.routingNumber) {
throw new Error('Invalid bank details');
}
// Process with bank API
return {
success: true,
transactionId: 'bt_' + Date.now()
};
}

processCryptoPayment(payment) {
console.log('Processing crypto payment');
// Validate wallet address
if (!payment.walletAddress) {
throw new Error('Invalid wallet address');
}
// Process with crypto API
return {
success: true,
transactionId: 'crypto_' + Date.now()
};
}
}
Polymorphic ApproachRecommended
// Using polymorphism and strategy pattern
// Base payment processor interface
class PaymentProcessor {
processPayment(payment) {
throw new Error('This method should be implemented by subclasses');
}
}

// Specific payment processor implementations
class CreditCardProcessor extends PaymentProcessor {
processPayment(payment) {
console.log('Processing credit card payment');
// Validate credit card details
if (!payment.cardNumber || !payment.expiryDate || !payment.cvv) {
throw new Error('Invalid credit card details');
}
// Process with credit card gateway
return {
success: true,
transactionId: 'cc_' + Date.now()
};
}
}

class PayPalProcessor extends PaymentProcessor {
processPayment(payment) {
console.log('Processing PayPal payment');
// Validate PayPal details
if (!payment.email) {
throw new Error('Invalid PayPal email');
}
// Process with PayPal API
return {
success: true,
transactionId: 'pp_' + Date.now()
};
}
}

class BankTransferProcessor extends PaymentProcessor {
processPayment(payment) {
console.log('Processing bank transfer');
// Validate bank details
if (!payment.accountNumber || !payment.routingNumber) {
throw new Error('Invalid bank details');
}
// Process with bank API
return {
success: true,
transactionId: 'bt_' + Date.now()
};
}
}

class CryptoProcessor extends PaymentProcessor {
processPayment(payment) {
console.log('Processing crypto payment');
// Validate wallet address
if (!payment.walletAddress) {
throw new Error('Invalid wallet address');
}
// Process with crypto API
return {
success: true,
transactionId: 'crypto_' + Date.now()
};
}
}

// Factory for getting the right processor
class PaymentProcessorFactory {
static getProcessor(paymentType) {
switch(paymentType) {
case 'credit_card':
return new CreditCardProcessor();
case 'paypal':
return new PayPalProcessor();
case 'bank_transfer':
return new BankTransferProcessor();
case 'crypto':
return new CryptoProcessor();
default:
throw new Error(`Unsupported payment type: ${paymentType}`);
}
}
}

// Usage
function processPayment(payment) {
const processor = PaymentProcessorFactory.getProcessor(payment.type);
return processor.processPayment(payment);
}

Change Preventers

These smells make the code difficult to change:

Change Preventer Smells

🔀

Divergent Change

When a class is modified for different reasons in different ways

Example: A User class that changes when the authentication system, permission system, or profile system changes
🔫

Shotgun Surgery

When a single change requires multiple modifications across classes

Example: Adding a new field requires changes to model, validator, form, database, and API classes
🌲

Parallel Inheritance Hierarchies

When creating a subclass in one hierarchy forces creating a subclass in another

Example: Every time you add a new Shape class, you must add a corresponding ShapeRenderer class

Divergent Change Refactoring

Splitting a class that changes for different reasons

Monolithic User ClassAvoid
// User class that changes for many different reasons
class User {
constructor(id, username, email, password) {
this.id = id;
this.username = username;
this.email = email;
this.password = password; // Hashed password
this.isActive = false;
this.activationToken = null;
this.profilePicture = null;
this.bio = '';
this.createdAt = new Date();
this.lastLogin = null;
this.failedLoginAttempts = 0;
this.lockedUntil = null;
this.roles = ['user'];
this.permissions = [];
this.notificationPreferences = {
email: true,
sms: false,
push: true
};
this.paymentMethods = [];
this.subscription = null;
this.timezone = 'UTC';
this.language = 'en';
}

// Authentication methods
validatePassword(password) { /* ... */ }
resetPassword(newPassword) { /* ... */ }
generateResetToken() { /* ... */ }
activate(token) { /* ... */ }
lock() { /* ... */ }
unlock() { /* ... */ }
recordLoginAttempt(success) { /* ... */ }

// Profile methods
updateProfile(profileData) { /* ... */ }
uploadProfilePicture(image) { /* ... */ }

// Permission methods
hasPermission(permission) { /* ... */ }
assignRole(role) { /* ... */ }
revokeRole(role) { /* ... */ }

// Notification methods
updateNotificationPreferences(preferences) { /* ... */ }
sendNotification(message) { /* ... */ }

// Subscription methods
subscribe(plan) { /* ... */ }
unsubscribe() { /* ... */ }
addPaymentMethod(paymentDetails) { /* ... */ }
removePaymentMethod(paymentId) { /* ... */ }

// Settings methods
updateSettings(settings) { /* ... */ }
}
Single Responsibility ClassesRecommended
// Split into multiple focused classes with single responsibilities

// Core user identity
class User {
constructor(id, username, email) {
this.id = id;
this.username = username;
this.email = email;
this.createdAt = new Date();
}
}

// Authentication concerns
class UserAuthentication {
constructor(userId, passwordHash) {
this.userId = userId;
this.passwordHash = passwordHash;
this.isActive = false;
this.activationToken = null;
this.lastLogin = null;
this.failedLoginAttempts = 0;
this.lockedUntil = null;
}

validatePassword(password) { /* ... */ }
resetPassword(newPassword) { /* ... */ }
generateResetToken() { /* ... */ }
activate(token) { /* ... */ }
lock() { /* ... */ }
unlock() { /* ... */ }
recordLoginAttempt(success) { /* ... */ }
}

// User profile information
class UserProfile {
constructor(userId) {
this.userId = userId;
this.profilePicture = null;
this.bio = '';
this.timezone = 'UTC';
this.language = 'en';
}

updateProfile(profileData) { /* ... */ }
uploadProfilePicture(image) { /* ... */ }
updateSettings(settings) { /* ... */ }
}

// Permissions and access control
class UserPermissions {
constructor(userId) {
this.userId = userId;
this.roles = ['user'];
this.permissions = [];
}

hasPermission(permission) { /* ... */ }
assignRole(role) { /* ... */ }
revokeRole(role) { /* ... */ }
}

// Notification preferences and sending
class UserNotifications {
constructor(userId) {
this.userId = userId;
this.preferences = {
email: true,
sms: false,
push: true
};
}

updatePreferences(preferences) { /* ... */ }
sendNotification(message) { /* ... */ }
}

// Billing and subscriptions
class UserSubscription {
constructor(userId) {
this.userId = userId;
this.paymentMethods = [];
this.subscription = null;
}

subscribe(plan) { /* ... */ }
unsubscribe() { /* ... */ }
addPaymentMethod(paymentDetails) { /* ... */ }
removePaymentMethod(paymentId) { /* ... */ }
}

// Facade for client code that prefers a unified interface
class UserFacade {
constructor(userId) {
this.user = userRepository.getUser(userId);
this.auth = authRepository.getAuthentication(userId);
this.profile = profileRepository.getProfile(userId);
this.permissions = permissionRepository.getPermissions(userId);
this.notifications = notificationRepository.getNotifications(userId);
this.subscription = subscriptionRepository.getSubscription(userId);
}

// Delegate methods for client code convenience
// ...
}

Dispensables

These smells indicate code that is unnecessary and should be removed:

Dispensable Code Smells

💬

Comments

Excessive comments often indicate unclear code that should be rewritten

Example: A method with several paragraphs of comments explaining its complex behavior
📋

Duplicate Code

The same or similar code appears in multiple places

Example: Validation logic duplicated across multiple form handlers
⚰️

Dead Code

Code that is never executed and serves no purpose

Example: Methods that are never called or conditional branches that are never reached
🔮

Speculative Generality

Code written for 'someday' that isn't actually needed now

Example: An overly complex, flexible framework for a simple feature that may expand in the future

Duplicate Code Refactoring

Extracting common functionality from duplicated code

Duplicated Validation LogicAvoid
// Duplicate validation logic across multiple forms
class RegistrationForm {
validateEmail(email) {
if (!email) {
return 'Email is required';
}
if (!email.includes('@')) {
return 'Email must be valid';
}
if (email.length > 255) {
return 'Email is too long';
}
return null;
}

validatePassword(password) {
if (!password) {
return 'Password is required';
}
if (password.length < 8) {
return 'Password must be at least 8 characters';
}
if (!/[A-Z]/.test(password)) {
return 'Password must contain an uppercase letter';
}
if (!/[0-9]/.test(password)) {
return 'Password must contain a number';
}
return null;
}

// Other registration form methods...
}

class ProfileForm {
validateEmail(email) {
if (!email) {
return 'Email is required';
}
if (!email.includes('@')) {
return 'Email must be valid';
}
if (email.length > 255) {
return 'Email is too long';
}
return null;
}

validateBio(bio) {
if (bio && bio.length > 1000) {
return 'Bio cannot exceed 1000 characters';
}
return null;
}

// Other profile form methods...
}

class LoginForm {
validateEmail(email) {
if (!email) {
return 'Email is required';
}
if (!email.includes('@')) {
return 'Email must be valid';
}
if (email.length > 255) {
return 'Email is too long';
}
return null;
}

validatePassword(password) {
if (!password) {
return 'Password is required';
}
return null;
}

// Other login form methods...
}
Centralized ValidationRecommended
// Centralized validation logic
class Validator {
static email(email) {
if (!email) {
return 'Email is required';
}
if (!email.includes('@')) {
return 'Email must be valid';
}
if (email.length > 255) {
return 'Email is too long';
}
return null;
}

static password(password, options = { requireStrong: true }) {
if (!password) {
return 'Password is required';
}

// Skip additional checks for non-strong passwords (e.g., login form)
if (!options.requireStrong) {
return null;
}

if (password.length < 8) {
return 'Password must be at least 8 characters';
}
if (!/[A-Z]/.test(password)) {
return 'Password must contain an uppercase letter';
}
if (!/[0-9]/.test(password)) {
return 'Password must contain a number';
}
return null;
}

static bio(bio) {
if (bio && bio.length > 1000) {
return 'Bio cannot exceed 1000 characters';
}
return null;
}

// Other validation methods...
}

// Forms using centralized validation
class RegistrationForm {
validateEmail(email) {
return Validator.email(email);
}

validatePassword(password) {
return Validator.password(password);
}

// Other registration form methods...
}

class ProfileForm {
validateEmail(email) {
return Validator.email(email);
}

validateBio(bio) {
return Validator.bio(bio);
}

// Other profile form methods...
}

class LoginForm {
validateEmail(email) {
return Validator.email(email);
}

validatePassword(password) {
return Validator.password(password, { requireStrong: false });
}

// Other login form methods...
}

Couplers

These smells indicate excessive coupling between classes:

Coupler Code Smells

😍

Feature Envy

A method that seems more interested in another class than the one it's in

Example: A method in OrderProcessor that mostly uses Customer data and methods
👫

Inappropriate Intimacy

Classes that are too tightly coupled and know too much about each other

Example: Two classes that directly access each other's private fields
⛓️

Message Chains

A series of method calls to navigate to the desired object

Example: client.getOrder().getCustomer().getAddress().getCountry()
👨‍💼

Middle Man

A class that delegates most of its work to another class

Example: A 'UserManager' class with methods that simply pass through to 'UserRepository'

Feature Envy Refactoring

Moving a method to the class it's most interested in

Feature EnvyAvoid
// Method more interested in Order than its own class
class OrderAnalytics {
calculateOrderTotal(order) {
let total = 0;

// Add up item prices
for (const item of order.items) {
const itemPrice = item.quantity * item.unitPrice;
total += itemPrice;
}

// Apply discounts
if (order.discountCode) {
const discount = this.getDiscountAmount(order.discountCode, total);
total -= discount;
}

// Add shipping cost
if (order.expeditedShipping) {
total += order.items.length * 2; // $2 per item for expedited
} else if (total < 50) {
total += 5; // Standard $5 shipping for orders under $50
}

// Apply tax
const taxRate = this.getTaxRate(order.shippingAddress.zipCode);
const tax = total * taxRate;
total += tax;

return total;
}

getDiscountAmount(discountCode, subtotal) {
// Discount logic
return 0;
}

getTaxRate(zipCode) {
// Tax logic based on location
return 0.08; // Default 8%
}

// Other analytics methods...
analyzeOrderTrends() { /* ... */ }
generateOrderReports() { /* ... */ }
}
Method in Proper ClassRecommended
// Order class with total calculation method
class Order {
constructor(items, discountCode, expeditedShipping, shippingAddress) {
this.items = items;
this.discountCode = discountCode;
this.expeditedShipping = expeditedShipping;
this.shippingAddress = shippingAddress;
}

calculateTotal(discountService, taxService) {
let total = 0;

// Add up item prices
for (const item of this.items) {
const itemPrice = item.quantity * item.unitPrice;
total += itemPrice;
}

// Apply discounts
if (this.discountCode) {
const discount = discountService.getDiscountAmount(this.discountCode, total);
total -= discount;
}

// Add shipping cost
if (this.expeditedShipping) {
total += this.items.length * 2; // $2 per item for expedited
} else if (total < 50) {
total += 5; // Standard $5 shipping for orders under $50
}

// Apply tax
const taxRate = taxService.getTaxRate(this.shippingAddress.zipCode);
const tax = total * taxRate;
total += tax;

return total;
}

// Other order methods...
}

// Services for discounts and taxes
class DiscountService {
getDiscountAmount(discountCode, subtotal) {
// Discount logic
return 0;
}
}

class TaxService {
getTaxRate(zipCode) {
// Tax logic based on location
return 0.08; // Default 8%
}
}

// Analytics class uses Order's methods rather than doing the work itself
class OrderAnalytics {
constructor(discountService, taxService) {
this.discountService = discountService;
this.taxService = taxService;
}

// Now OrderAnalytics doesn't need to know how to calculate totals
analyzeOrderTrends(orders) {
const orderTotals = orders.map(order =>
order.calculateTotal(this.discountService, this.taxService)
);
// Analyze trends with totals
return {
averageOrderValue: this.calculateAverage(orderTotals),
// Other metrics...
};
}

generateOrderReports() { /* ... */ }

calculateAverage(values) {
const sum = values.reduce((total, value) => total + value, 0);
return sum / values.length;
}
}

Refactoring Techniques

Refactoring

Like renovating a house while keeping its exterior appearance the same. You improve the internal structure, organization, and systems without changing what the house fundamentally is or does.

Extract Method

Extract Method Refactoring

Creating a new method from a code fragment

Long MethodAvoid
function renderUserProfile(user) {
// Render user details
let html = '<div class="user-profile">';
html += `<h2>${user.name}</h2>`;
html += `<p>${user.bio || 'No bio provided'}</p>`;

// Calculate and render user activity stats
let totalPosts = 0;
let totalComments = 0;
let totalLikes = 0;

for (const activity of user.activities) {
if (activity.type === 'post') {
totalPosts++;
} else if (activity.type === 'comment') {
totalComments++;
} else if (activity.type === 'like') {
totalLikes++;
}
}

html += '<div class="activity-stats">';
html += `<div class="stat">Posts: ${totalPosts}</div>`;
html += `<div class="stat">Comments: ${totalComments}</div>`;
html += `<div class="stat">Likes: ${totalLikes}</div>`;
html += '</div>';

// Render user badges
html += '<div class="user-badges">';
for (const badge of user.badges) {
html += `<span class="badge ${badge.type}">${badge.name}</span>`;
}
html += '</div>';

html += '</div>';
return html;
}
Extracted MethodsRecommended
function renderUserProfile(user) {
let html = '<div class="user-profile">';
html += renderUserDetails(user);
html += renderActivityStats(user.activities);
html += renderUserBadges(user.badges);
html += '</div>';
return html;
}

function renderUserDetails(user) {
return `
<h2>${user.name}</h2>
<p>${user.bio || 'No bio provided'}</p>
`;
}

function renderActivityStats(activities) {
const stats = calculateActivityStats(activities);

return `
<div class="activity-stats">
<div class="stat">Posts: ${stats.posts}</div>
<div class="stat">Comments: ${stats.comments}</div>
<div class="stat">Likes: ${stats.likes}</div>
</div>
`;
}

function calculateActivityStats(activities) {
let posts = 0, comments = 0, likes = 0;

for (const activity of activities) {
if (activity.type === 'post') {
posts++;
} else if (activity.type === 'comment') {
comments++;
} else if (activity.type === 'like') {
likes++;
}
}

return { posts, comments, likes };
}

function renderUserBadges(badges) {
let html = '<div class="user-badges">';
for (const badge of badges) {
html += `<span class="badge ${badge.type}">${badge.name}</span>`;
}
html += '</div>';
return html;
}

Move Method

Move Method Refactoring

Moving a method to a more appropriate class

Method in Wrong ClassAvoid
// Method in the wrong class
class Customer {
constructor(name, plan) {
this.name = name;
this.plan = plan;
}

getPlanDiscount() {
// This method knows too much about Plan details
switch(this.plan.type) {
case 'basic':
return 0;
case 'premium':
return 0.1; // 10% discount
case 'gold':
return 0.15; // 15% discount
case 'platinum':
if (this.plan.years > 2) {
return 0.25; // 25% for long-term platinum members
}
return 0.2; // 20% regular platinum discount
default:
return 0;
}
}
}

class Plan {
constructor(type, years) {
this.type = type;
this.years = years;
}
}
Method in Proper ClassRecommended
// Method moved to its proper class
class Customer {
constructor(name, plan) {
this.name = name;
this.plan = plan;
}

getPlanDiscount() {
// Now delegates to Plan, which knows its own details
return this.plan.getDiscount();
}
}

class Plan {
constructor(type, years) {
this.type = type;
this.years = years;
}

getDiscount() {
switch(this.type) {
case 'basic':
return 0;
case 'premium':
return 0.1; // 10% discount
case 'gold':
return 0.15; // 15% discount
case 'platinum':
if (this.years > 2) {
return 0.25; // 25% for long-term platinum members
}
return 0.2; // 20% regular platinum discount
default:
return 0;
}
}
}

Replace Conditional with Polymorphism

Replace Conditional with Polymorphism

Using class inheritance instead of conditionals

Conditional LogicAvoid
// Employee salary calculation with conditional logic
class Employee {
constructor(name, type, hourlyRate, monthlySalary, commissionRate, salesAmount) {
this.name = name;
this.type = type; // 'hourly', 'salaried', or 'commissioned'
this.hourlyRate = hourlyRate;
this.monthlySalary = monthlySalary;
this.commissionRate = commissionRate;
this.salesAmount = salesAmount;
}

calculatePay() {
switch(this.type) {
case 'hourly':
return this.hourlyRate * 160; // Assuming 160 hours per month
case 'salaried':
return this.monthlySalary;
case 'commissioned':
return this.monthlySalary + (this.commissionRate * this.salesAmount);
default:
throw new Error(`Invalid employee type: ${this.type}`);
}
}
}
Polymorphic ClassesRecommended
// Base employee class
class Employee {
constructor(name) {
this.name = name;
}

calculatePay() {
throw new Error('This method should be implemented by subclasses');
}
}

// Specialized employee classes
class HourlyEmployee extends Employee {
constructor(name, hourlyRate) {
super(name);
this.hourlyRate = hourlyRate;
}

calculatePay() {
return this.hourlyRate * 160; // Assuming 160 hours per month
}
}

class SalariedEmployee extends Employee {
constructor(name, monthlySalary) {
super(name);
this.monthlySalary = monthlySalary;
}

calculatePay() {
return this.monthlySalary;
}
}

class CommissionedEmployee extends Employee {
constructor(name, monthlySalary, commissionRate, salesAmount) {
super(name);
this.monthlySalary = monthlySalary;
this.commissionRate = commissionRate;
this.salesAmount = salesAmount;
}

calculatePay() {
return this.monthlySalary + (this.commissionRate * this.salesAmount);
}
}

Introduce Parameter Object

Introduce Parameter Object

Replacing a group of parameters with an object

Long Parameter ListAvoid
// Function with many parameters
function createBooking(customerName, customerEmail, customerPhone,
roomType, checkInDate, checkOutDate,
numberOfGuests, specialRequests,
paymentMethod, paymentAmount) {
// Validate customer details
if (!customerName || !customerEmail) {
throw new Error('Customer name and email are required');
}

// Validate room booking
if (!roomType || !checkInDate || !checkOutDate || numberOfGuests <= 0) {
throw new Error('Invalid room booking details');
}

// Validate payment
if (!paymentMethod || paymentAmount <= 0) {
throw new Error('Invalid payment details');
}

// Create booking logic...
return {
id: generateBookingId(),
customer: {
name: customerName,
email: customerEmail,
phone: customerPhone
},
room: {
type: roomType,
checkIn: checkInDate,
checkOut: checkOutDate,
guests: numberOfGuests,
requests: specialRequests
},
payment: {
method: paymentMethod,
amount: paymentAmount
}
};
}
Parameter ObjectsRecommended
// Using parameter objects
function createBooking(customerDetails, roomBooking, paymentDetails) {
// Validate customer details
if (!customerDetails.name || !customerDetails.email) {
throw new Error('Customer name and email are required');
}

// Validate room booking
if (!roomBooking.type || !roomBooking.checkInDate ||
!roomBooking.checkOutDate || roomBooking.numberOfGuests <= 0) {
throw new Error('Invalid room booking details');
}

// Validate payment
if (!paymentDetails.method || paymentDetails.amount <= 0) {
throw new Error('Invalid payment details');
}

// Create booking logic...
return {
id: generateBookingId(),
customer: {
name: customerDetails.name,
email: customerDetails.email,
phone: customerDetails.phone
},
room: {
type: roomBooking.type,
checkIn: roomBooking.checkInDate,
checkOut: roomBooking.checkOutDate,
guests: roomBooking.numberOfGuests,
requests: roomBooking.specialRequests
},
payment: {
method: paymentDetails.method,
amount: paymentDetails.amount
}
};
}

// Usage
createBooking(
{
name: 'Jane Smith',
email: 'jane@example.com',
phone: '555-1234'
},
{
type: 'deluxe',
checkInDate: new Date('2023-05-15'),
checkOutDate: new Date('2023-05-20'),
numberOfGuests: 2,
specialRequests: 'High floor, away from elevator'
},
{
method: 'credit_card',
amount: 750.00
}
);

Refactoring Process

Refactoring Process

Like restoring an old house one room at a time, making small, manageable improvements while ensuring the house remains functional throughout the process.

Best Practices for Refactoring

  1. Have tests in place: Never refactor without automated tests to verify behavior
  2. Take small steps: Make one small change at a time
  3. Test after each change: Run tests frequently to catch issues early
  4. Don't mix refactoring with new features: Separate improving structure from adding functionality
  5. Use tools: Leverage IDE refactoring tools when available
  6. Document why: Leave comments or documentation about why a refactoring was done

Tools for Detecting Code Smells

Several tools can help identify code smells automatically:

  • Static analyzers: SonarQube, ESLint, StyleCop, Checkstyle
  • Complexity analyzers: Metrics such as cyclomatic complexity and cognitive complexity
  • Duplicate code detectors: PMD CPD, Simian, JScpd
  • Architecture validators: ArchUnit, NDepend, Structure101

Conclusion

Code smells are valuable indicators that help developers identify areas of code that could benefit from refactoring. By learning to recognize these patterns and applying appropriate refactoring techniques, you can gradually improve your codebase's quality, maintainability, and flexibility.

Remember that refactoring is not a one-time activity but an ongoing part of software development. Regular, small refactorings prevent technical debt from accumulating and keep your codebase healthy over time.

As Martin Fowler says, "Refactoring is a disciplined technique for restructuring an existing body of code, altering its internal structure without changing its external behavior." This discipline, applied consistently, is what transforms a problematic codebase into a clean, maintainable one.