Cracking The Code Smells
How to detect code smells and refactor our codebase to a cleaner and leaner code!
Intro
Coding is like a skilled craft, making our code functional and clear. Just as a detective finds hidden truths, “code smells” reveal issues in our code. Think of them as warning signs. This journey explores these code smells, showing their impact, real examples, and fixes. By mastering this, we improve our skills. In this article, we cover all code smells, with TypeScript code snippets, to help us become better developers. 🚀🔍
Poor Naming 📛
The Poor Naming convention is one of the most common and critical code smells. Imagine trying to solve a puzzle with missing pieces. That’s how it feels when we encounter unclear and confusing names in code. These are the most common poor-name candidates:
- Meaningless Variable Names
// Poor Naming
const x = 42;
const data = ['apple', 'orange', 'banana'];
// Better Naming
const age = 42;
const fruitList = ['apple', 'orange', 'banana'];- Ambiguous Function Names
// Poor Naming
function doSomething(a, b) {
// ...
}
// Better Naming
function calculateSum(num1, num2) {
// ...
}- Misleading Class Names
// Poor Naming
class HandleData {
// ...
}
// Better Naming
class DataManager {
// ...
}- Inconsistent Naming Styles
// Poor Naming
const maxVal = 100;
function calculate_value() {
// ...
}
// Better Naming
const maxValue = 100;
function calculateValue() {
// ...
}- Names WIth Encoding
// Poor Naming
const iMaxRequest: number = 100;
// Better Naming
const maxRequest: number = 100;- Noisy Names
// Poor Naming
const listOfAcceptedApplicants: Array<Applicant>;
// Better Naming
const acceptedApplicants: Array<Applicant>;Boolean Blindness 🔍
This code smell involves using unclear boolean variables or conditions that shroud logic in obscurity. Boolean Blindness can manifest in various forms, each impacting code readability and maintainability differently. Here are a few common types:
- Obscure Variable Naming: Using ambiguous names for boolean variables, making it challenging to understand their purpose and significance.
// Code Smell: Obscure Variable Naming
const isTrue = someCondition;
const hasPermission = canAccessResource;
// Improved: Clear Variable Naming
const isLoggedIn = someCondition;
const userHasAccess = canAccessResource;- Complex Negations: Nesting negations within conditions can lead to confusion, making it difficult to discern the intended logic.
// Code Smell: Complex Negations
if (!isNotAuthorized && (isEmployee || isAdmin)) {
// ...
}
// Improved: Simplified Logic
if (isAuthorized && (isEmployee || isAdmin)) {
// ...
}- Overloaded Boolean Logic: Squeezing too many conditions into a single boolean expression can make the code convoluted and difficult to follow.
// Code Smell: Overloaded Boolean Logic
if (isWeekend && (isHoliday || isWeatherGood) && (hasEnoughFuel || isCarElectric)) {
// ...
}
// Improved: Clear Conditions
const isOptimalWeather = isHoliday || isWeatherGood;
const canTravel = hasEnoughFuel || isCarElectric;
if (isWeekend && isOptimalWeather && canTravel) {
// ...
}- Magic Booleans: Relying on literal boolean values like
trueorfalsewithout explanatory context can be puzzling, especially when they don't reveal the reasoning behind decisions.
// Code Smell: Magic Booleans
if (isAllowed) {
// ...
}
// Improved: Descriptive Naming
const userIsAuthorized = isAllowed;
if (userIsAuthorized) {
// ...
}- Inconsistent Usage: Inconsistently applying boolean variables or conditions throughout the codebase can create cognitive dissonance, impairing understanding.
// Code Smell: Inconsistent Usage
if (isEnabled) {
// ...
}
// Elsewhere in the code...
if (isActive) {
// ...
}
// Improved: Consistent Usage
if (userIsEnabled) {
// ...
}
// Elsewhere in the code...
if (userIsActive) {
// ...
}Code Duplication 🖨️
Imagine writing a story but repeating the same sentences over and over. That’s what happens in coding with Code Duplication. It’s like copying and pasting the same words, which might seem easy, but it creates confusion and messiness.
// Code Duplication: Repeated Logic
function calculateTotalPrice(price: number, quantity: number): number {
return price * quantity;
}
function calculateTaxedPrice(price: number, quantity: number): number {
const taxRate = 0.1;
return (price * quantity) + (price * quantity * taxRate);
}By using the calculateTotalPrice function within calculateTaxedPrice, we can eliminate the repetition and keep our code clean and clear.
// Improved: Reusable Logic
function calculateTotalPrice(price: number, quantity: number): number {
return price * quantity;
}
function calculateTaxedPrice(price: number, quantity: number): number {
const taxRate = 0.1;
const total = calculateTotalPrice(price, quantity);
return total + (total * taxRate);
}Long Method and Function 🕰️
These are like having a never-ending recipe with too many steps — it’s hard to follow and understand.
// Long Method Code Smell: Complex Calculation
function calculateTotalPrice(items: Array<Item>): number {
let totalPrice = 0;
for (const item of items) {
totalPrice += item.price * item.quantity;
}
return totalPrice;
}Let’s make this code shorter:
// Improved: Shorter Method with Helper Functions
function calculateTotalPrice(items: Array<Item>): number {
return items.reduce((total, item) => total + calculateItemTotal(item), 0);
}
function calculateItemTotal(item: Item): number {
return item.price * item.quantity;
}Spaghetti Code 🍝
Imagine your computer code is like a big plate of tangled spaghetti. We call this Spaghetti Code. It’s when the code gets all mixed up and messy, making it really hard to figure out what’s going on. When code becomes like spaghetti, it’s tough to change or fix things because everything is so jumbled up.
// Spaghetti Code: Complex Flow
function processCustomerOrder(customer: Customer, products: Array<Product>): boolean {
let orderIsValid = false;
if (customer.status === 'active') {
if (customer.credit > 1000) {
for (const product of products) {
if (product.stock > 0) {
orderIsValid = true;
product.stock--;
} else {
orderIsValid = false;
break;
}
}
} else {
orderIsValid = false;
}
} else {
orderIsValid = false;
}
return orderIsValid;
}Improved Code with Simplified Logic:
// Improved: Simplified Logic
function processCustomerOrder(customer: Customer, products: Array<Product>): boolean {
if (isCustomerEligible(customer)) {
return tryPlaceOrder(products);
}
return false;
}
function isCustomerEligible(customer: Customer): boolean {
return customer.status === 'active' && customer.credit > 1000;
}
function tryPlaceOrder(products: Array<Product>): boolean {
for (const product of products) {
if (isProductAvailable(product)) {
product.stock--;
} else {
return false;
}
}
return true;
}
function isProductAvailable(product: Product): boolean {
return product.stock > 0;
}Primitive Obsession 🛠️
Imagine using the same tool for everything, like a Swiss Army knife — it might work, but it’s not always the best choice. In coding, we encounter a similar situation with Primitive Obsession. This happens when we rely too much on basic data types like numbers and strings, leading to messy and less flexible code.
// Primitive Obsession: Overused Primitive
class Order {
private id: number;
private customerName: string;
private totalAmount: number;
constructor(id: number, customerName: string, totalAmount: number) {
this.id = id;
this.customerName = customerName;
this.totalAmount = totalAmount;
}
// Methods related to orders...
}Here, we’re using primitive types for key aspects of an order. It’s like trying to build a skyscraper with only a hammer! Instead, we can create dedicated classes:
// Improved: Specialized Classes
class OrderId {
private value: number;
constructor(value: number) {
this.value = value;
}
// Methods related to order IDs...
}
class CustomerName {
private value: string;
constructor(value: string) {
this.value = value;
}
// Methods related to customer names...
}
class MoneyAmount {
private value: number;
constructor(value: number) {
this.value = value;
}
// Methods related to money amounts...
}
class Order {
private id: OrderId;
private customerName: CustomerName;
private totalAmount: MoneyAmount;
constructor(id: OrderId, customerName: CustomerName, totalAmount: MoneyAmount) {
this.id = id;
this.customerName = customerName;
this.totalAmount = totalAmount;
}
// Methods related to orders...
}Comments and Documentation Code Smells 📝
Think of it like writing a story but having to add explanations on every page so people can understand it. Just as doctors can read exam stats without the full report, programmers aim for code so clear that comments aren’t needed — the code itself should tell the tale.
// Comments and Documentation Code Smells: Over-Commented Code
function calculateTotal(items: Array<Item>): number {
// Loop through items and calculate total
let total = 0;
for (const item of items) {
total += item.price * item.quantity;
}
return total; // Return the calculated total
}Here, we have comments explaining every step — like a recipe with notes for each ingredient! Instead, let’s write self-explanatory code:
// Improved: Self-Explanatory Code
function calculateTotal(items: Array<Item>): number {
let total = 0;
for (const item of items) {
total += item.price * item.quantity;
}
return total;
}Feature Envy 👀
Feature Envy is like having one part of your code always snooping around another’s work. In coding, it’s a code smell where a class seems overly interested in the data of another class. This can make the code less organized and harder to maintain.
// Feature Envy Code Smell: Overly Interested Method
class ShoppingCart {
private items: Array<Item>= [];
// ...
calculateTotalPrice(): number {
let totalPrice = 0;
for (const item of this.items) {
totalPrice += item.price * item.quantity;
}
return totalPrice;
}
isAffordable(): boolean {
const total = this.calculateTotalPrice();
return total <= 100;
}
}Here, the isAffordable method is more interested in the calculateTotalPrice method's data than its own. Let's find a better balance:
// Improved: Balanced and Encapsulated Code
class ShoppingCart {
private items: Array<Item> = [];
// ...
calculateTotalPrice(): number {
let totalPrice = 0;
for (const item of this.items) {
totalPrice += item.price * item.quantity;
}
return totalPrice;
}
}
class PriceEvaluator {
private shoppingCart: ShoppingCart;
constructor(cart: ShoppingCart) {
this.shoppingCart = cart;
}
isAffordable(): boolean {
const total = this.shoppingCart.calculateTotalPrice();
return total <= 100;
}
}Data Clumps 🧶
Imagine having a bag where you keep all your belongings mixed up — it’s hard to find what you need. In coding, we encounter a similar situation with Data Clumps. This is like having related data scattered across your code, making it harder to manage and understand.
// Data Clumps Code Smell: Scattered Data
function calculateTotal(order: Order): number {
const basePrice = order.items.reduce((total, item) => total + item.price, 0);
const tax = basePrice * 0.1; // 10% tax rate
return basePrice + tax;
}Here, the base price and tax rate are separate but related pieces of data — it’s like storing your shoes in one room and socks in another. Let’s bundle them together:
// Improved: Grouped Data
class Order {
private items: Array<Item>;
private taxRate: number;
constructor(items: Item[], taxRate: number) {
this.items = items;
this.taxRate = taxRate;
}
calculateTotal(): number {
const basePrice = this.items.reduce((total, item) => total + item.price, 0);
const tax = basePrice * this.taxRate;
return basePrice + tax;
}
}By organizing related data into a single structure, we make our code more cohesive and easier to maintain.
Inappropriate Intimacy 👥
Imagine someone you just met asking for personal details — it can feel uncomfortable and invasive. In coding, we encounter a similar issue with Inappropriate Intimacy. This is like code that’s too closely connected to another class, making it harder to manage and leading to tangled relationships.
// Inappropriate Intimacy Code Smell: Tight Coupling
class Customer {
private name: string;
// ...
getInvoice(): Invoice {
return new Invoice(this);
}
}
class Invoice {
private customer: Customer;
constructor(customer: Customer) {
this.customer = customer;
}
// ...
}Here, the Invoice class is overly interested in the Customer class—it's like asking someone for their life story after just meeting them. Let's respect personal space:
// Improved: Looser Coupling
class Customer {
private name: string;
// ...
createInvoice(): Invoice {
return new Invoice(this);
}
}
class Invoice {
private customerName: string;
constructor(customer: Customer) {
this.customerName = customer.getName();
}
// ...
}Shotgun Surgery 🔫
Imagine needing a single small change, but you have to operate on your whole body — that’s not efficient. In coding, we encounter a similar situation with Shotgun Surgery. This is like making minor updates that require changes in many places, leading to a messy and error-prone codebase.
// Shotgun Surgery Code Smell: Spread-Out Changes
class Account {
// ...
updateBalance(amount: number): void {
this.balance += amount;
this.updateLastTransaction();
this.notifyAccountHolder();
}
updateLastTransaction(): void {
// ...
}
notifyAccountHolder(): void {
// ...
}
}Here, a small change to the balance requires updates in multiple methods — it’s like getting a haircut and needing to change your entire wardrobe. Let’s use a scalpel instead:
// Improved: Centralized Change
class Account {
// ...
updateBalance(amount: number): void {
this.balance += amount;
this.processTransaction();
}
processTransaction(): void {
this.updateLastTransaction();
this.notifyAccountHolder();
}
updateLastTransaction(): void {
// ...
}
notifyAccountHolder(): void {
// ...
}
}By centralizing related changes into a single method, we make our codebase more maintainable and surgical.
Lazy Class and Dead Code 💤
In coding, we face a similar challenge with Lazy Class and Dead Code Smells. This occurs when we have classes that do very little or code that serves no purpose, cluttering our codebase and making it harder to maintain.
// Lazy Class Smell: Unnecessary Class
class EmailAddress {
private address: string;
constructor(address: string) {
this.address = address;
}
validate(): boolean {
// Validation logic
}
}
// Dead Code Smell: Unused Function
function calculateDiscount(price: number): number {
// Calculation logic
}
function main() {
const totalPrice = 200;
const discount = calculateDiscount(totalPrice); // Unused calculation
// ...
}
main();Here, we have a class that does very little and an unused function — it’s like keeping items you never use in your closet. Let’s tidy up:
// Improved: Streamlined Code
// EmailAddress class removed
// Unused Function Removed
function main() {
const totalPrice = 200;
// ...
}
main();Switch Statements, Cyclomatic Complexity 🌀
In coding, we come across a situation known as Switch Statements and Cyclomatic Complexity. This arises when our code becomes tangled with numerous choices or decision points, leading to challenges in comprehending and managing it.
// Switch Statements and Cyclomatic Complexity: Complex Control Flow
function getCategoryDiscount(item: Item, user: User): number {
switch (item.category) {
case 'electronics':
if (user.isPremium()) {
return 0.2;
} else {
return 0.1;
}
case 'clothing':
return 0.15;
case 'books':
return 0.05;
default:
return 0;
}
}Here, the code has multiple branches and nested conditions — it’s like trying to navigate a maze of roads. Let’s simplify the route:
// Improved: Simplified Control Flow
class DiscountCalculator {
private item: Item;
private user: User;
constructor(item: Item, user: User) {
this.item = item;
this.user = user;
}
calculateDiscount(): number {
if (this.item.category === 'electronics') {
return this.calculateElectronicsDiscount();
} else if (this.item.category === 'clothing') {
return 0.15;
} else if (this.item.category === 'books') {
return 0.05;
} else {
return 0;
}
}
private calculateElectronicsDiscount(): number {
if (this.user.isPremium()) {
return 0.2;
} else {
return 0.1;
}
}
}Large Parameter List 📥
In coding, we encounter a common issue known as the Large Parameter List code smell. This happens when functions or methods have a lot of input information, which can make them confusing and more likely to have mistakes.
// Large Parameter List Code Smell: Overwhelming Inputs
function createOrder(
product: Product,
quantity: number,
customer: Customer,
address: Address,
paymentMethod: PaymentMethod
) {
// Order creation logic...
}Here, the function has too many parameters — it’s like trying to carry a mountain of stuff in one go. Let’s unpack:
// Improved: Simplified Inputs
class Order {
constructor(
private product: Product,
private quantity: number,
private customer: Customer,
private address: Address,
private paymentMethod: PaymentMethod
) {
// Order creation logic...
}
}By encapsulating related parameters within a class, we create a more organized and manageable structure.
Temporary Fields 📦
In coding, we encounter a similar situation with the Temporary Fields code smell. This happens when objects have fields that are only used temporarily, cluttering the code and adding confusion.
// Temporary Fields Code Smell: Unnecessary Fields
class ShoppingCart {
private items: Item[] = [];
private tempSubtotal: number = 0; // Temporary field
addItem(item: Item): void {
this.items.push(item);
this.tempSubtotal += item.price;
}
getSubtotal(): number {
return this.tempSubtotal; // Return the temporary field
}
}Here, the tempSubtotal field is used temporarily—it's like carrying an extra bag for a short walk. Let's lighten the load:
// Improved: Streamlined Structure
class ShoppingCart {
private items: Item[] = [];
addItem(item: Item): void {
this.items.push(item);
}
calculateSubtotal(): number {
let subtotal = 0;
for (const item of this.items) {
subtotal += item.price;
}
return subtotal;
}
}By calculating the subtotal when needed instead of storing it, we create a more efficient and cleaner structure.
Parallel Inheritance Hierarchies 🏗️
In coding, we face a similar challenge with the Parallel Inheritance Hierarchies code smell. This occurs when two sets of classes have interdependent inheritance, making the code complex and harder to maintain.
// Parallel Inheritance Hierarchies Code Smell: Interdependent Hierarchies
class Employee {
// ...
}
class FullTimeEmployee extends Employee {
// ...
}
class ContractEmployee extends Employee {
// ...
}
class Payment {
// ...
}
class FullTimePayment extends Payment {
// ...
}
class ContractPayment extends Payment {
// ...
}Here, we have two sets of interdependent hierarchies — like having two separate road systems with no connecting bridges. Let’s bridge the gap:
// Improved: Aligned Hierarchies
class Employee {
// ...
getPayment(): Payment {
// Get payment logic...
}
}
class FullTimeEmployee extends Employee {
// ...
}
class ContractEmployee extends Employee {
// ...
}
class Payment {
// ...
}
class FullTimePayment extends Payment {
// ...
}
class ContractPayment extends Payment {
// ...
}By aligning the hierarchies with a common bridge, we create a more organized and interconnected structure.
Navigating Code Smells to Improve Your Craft 🌟
Learning about and fixing code smells is like having a guide to hidden treasures. As we’ve looked at different code smells, we’ve learned about problems in our code. But just knowing the issues isn’t enough — the real benefit comes from making improvements to make our code work better and be easier to maintain.
