Know Your Code Smells - Sharma Blogs

Summary#

So code smells is a term introduced by Kent Beck. Now generally people don’t know what code smells are.. and those who do.. when we ask them to list atleast 5 of them.. majority of the people are not able to.

Majority of us when we think of this term our mind goes like -> “I dont like your code, but I can’t tell you why!“. That is not what a code smell is.

In this blog we’ll read about

What a Code Smell
Classic Smells Categorised
Code Smells Dilemma

What a Code Smell is#

Code smells are any indicator that there’s a bigger problem with your code. They arise from a number of causes, including developer ignorance (i.e., a new developer or a not very good one) , rushed coding (someone just trying to “get the job done”), or unclear standards (company didn’t outline them for developers).

In any case, code smells are a sign that source code is messy or otherwise does not meet best practice standards in the developer world or at your organization.

Now all the classic 20 smells are listed below


Alternative Classes w/ different interfaces	Inappropriate Intimacy	Parallel Inheritance Hierarchies	Data Class	Large Class
Primitive Obsession	Data Clumps	Long Method	Shotgun Surgery	Divergent Change
Feature Envy	Middle Man	Temporary Field	Lazy Class	Refused Bequest
Long Parameter List	Speculative Generality	Duplicated Code	Message Chains	Switch Statements

Alternative Classes
w/ different interfaces

Inappropriate Intimacy

Parallel Inheritance
Hierarchies

Data Class

Large Class

Primitive Obsession

Data Clumps

Long Method

Shotgun Surgery

Divergent Change

Feature Envy

Middle Man

Temporary Field

Lazy Class

Refused Bequest

Long Parameter
List

Speculative Generality

Duplicated Code

Message Chains

Switch Statements

If your code adhere to or follow any of the above practices..then i must admit

Classic Smells Categorized#

Bloaters#

Large Class
Long Method
Data Clumps
Long Parameter List
Primitive Obsession

These are the list of things that really don't have to be too big

Long Methods and Long Classes size restriction are kind of self explanatory. These should be a predefined limit (as suggested by the linters), which everyone should adhere to.
Data clumps are 2 or more pieces of data that always appear together, hinting that they should probably be wrapped into their own object or class. For example, say you have three parameters: first_name, last_name, and address that keep popping up together in multiple places in your application. Every time you write a method or a class, you pass these three around like a group discount at a theme park. This is a sign you’re dealing with a data clump.
Long parameter list very obvious… Even though if it is occurring only once, it is always a good idea to wrap it in an object
Primitive Obsession is a very interesting concept as per me, This is where a lot of developers tend to overlook the problem. It happens when you rely too much on primitive data types (like integers, strings, booleans) to represent complex concepts or entities in your code. Instead of creating specific classes or types that better represent the domain, you keep using basic data types, which can make the code harder to understand and maintain and way bigger at places.
1. Using Strings or Integers for Everything: If you’re representing a PhoneNumber, Address, or Currency using a simple string or integer, that’s primitive obsession. These concepts have specific behaviors or formats, and using a primitive doesn’t capture the complexity properly.
  Here, @phone is just a string. You’ll end up repeatedly validating and formatting phone numbers all over your code, when it could be encapsulated in a PhoneNumber class.
```
class Customer
    def initialize(name, phone)
        @name = name
        @phone = phone  # phone is just a string
    end
end
```
1. Overusing Booleans: If your methods take a boolean parameter to control some behavior, it can lead to unreadable code and make it unclear what the parameter represents
```
def create_order(is_expedited)
    if is_expedited
        # expedited order logic
    else
        # regular order logic
    end
end
```
Here, it’s unclear what true or false means without looking at the method’s internals. Instead, you could create an OrderType or ShippingMode class.
1. Using Arrays or Hashes Instead of Objects: If you’re using a list or hash to store related properties rather than creating an object, you’re falling into this trap.
```
customer_info = ["John Doe", "123-456-7890", "123 Street, City"]
```
in this case, you’re relying on an array to represent a customer’s details instead of creating a Customer class. This makes the code more error-prone and harder to manage.

Tool Abusers#

Switch Statements
Refused Bequest
Alternative Classes w/ different interfaces
Temporary Field

These are tools present in Object Oriented Programming that can be misused very easily.

Switch Statements are a sign that your code may be avoiding object-oriented principles. When you find yourself with a giant switch (or if-else) statement, it’s often a sign that you’re handling different behaviors based on the type of object or value. Instead of sprawling case logic, you could use polymorphism to let each object handle its own behavior. Imagine a factory line where every machine operator has to read through a giant manual to figure out what to do next — that’s your switch statement.
```
case vehicle_type
when "car"
  # car logic
when "bike"
  # bike logic
else
  # default logic
end
```
A more flexible approach would be to encapsulate the logic within each class, so you avoid the manual lookup and have the machines just know what to do.
```
class Vehicle
  def drive
    raise NotImplementedError
  end
end

class Car < Vehicle
  def drive
    # car-specific logic
  end
end

class Bike < Vehicle
  def drive
    # bike-specific logic
  end
end
```
Refused Bequest occurs when a subclass inherits from a parent class but does not need or use much of the inherited functionality or raise an error on overriding a certain functionality. It’s like inheriting an old family business and refusing to run it because you want to do your own thing. The solution here is to rethink your inheritance structure and use composition or delegation instead, or perhaps create a more appropriate parent class. This smell often results from overusing inheritance without carefully considering whether it fits.
Alternative Classes with Different Interfaces arise when two or more classes perform similar tasks but have different methods or interfaces. This leads to inconsistencies, making the code harder to maintain and extend. Imagine trying to use a TV remote that has different buttons for volume on each brand of TV — frustrating, right? Instead, they should share the same interface, allowing clients to use them interchangeably.
```
class Dog
  def bark
    # dog-specific bark logic
  end
end

class Cat
  def meow
    # cat-specific meow logic
  end
end
```
Instead, they should share a common interface like speak, ensuring both animals can communicate without confusion for the client:
```
class Animal
  def speak
    raise NotImplementedError
  end
end

class Dog < Animal
  def speak
    # dog-specific bark logic
  end
end

class Cat < Animal
  def speak
    # cat-specific meow logic
  end
end
```
Temporary Field: This happens when an object has an instance variable that is only sometimes used, depending on a particular context or condition. It’s like carrying around an umbrella in case it rains, even though you live in the desert. Temporary fields often indicate that an object is trying to do too much or is responsible for too many concerns. A better approach would be to move that behavior to a class that needs it more consistently or to split the responsibilities into separate classes.
```
class Order
  def initialize(total_price, discount = nil)
    @total_price = total_price
    @discount = discount  # only used sometimes
  end

  def apply_discount
    @total_price -= @discount if @discount
  end
end
```
Instead of having a temporary field that’s sometimes used, it’s better to introduce a new class or strategy to handle discounted orders.
```
class DiscountedOrder
  def initialize(order, discount)
    @order = order
    @discount = discount
  end

  def apply_discount
    @order.total_price -= @discount
  end
end
```

By refactoring these tool abusers, we create cleaner, more maintainable code that avoids over-complication and promotes clarity.

Change Preventers#

Divergent Change
Shotgun Surgery
Parallel Inheritance Hierarchies

These entities are the core reason that prevent you from wanting to change code or make changes hard in your codebase.

Divergent Change occurs when one class is affected by many different kinds of changes for different reasons. It’s a sign that a class is doing too much — it’s like a Swiss Army knife that tries to handle everything but becomes cumbersome and hard to maintain. Each time you need to change one behavior, you risk breaking unrelated functionality. To avoid this, follow the Single Responsibility Principle by splitting the class into smaller, focused components that only need to change for one reason at a time.
```
class Customer
  def update_address
    # code to update address
  end

  def send_email
    # code to send email
  end

  def generate_invoice
    # code to generate invoice
  end
end
```
This class has too many responsibilities and can be broken down into smaller, specialized classes. One class might handle customer info, another handles communication, and yet another is responsible for billing.
```
class CustomerInfo
  def update_address
    # code to update address
  end
end

class CustomerCommunication
  def send_email
    # code to send email
  end
end

class Billing
  def generate_invoice
    # code to generate invoice
  end
end
```

Shotgun Surgery is a code smell that happens when a seemingly small change in your code requires you to make many little changes in multiple places throughout your codebase. It indicates that the logic is scattered across various classes or modules instead of being properly encapsulated. This makes the system hard to maintain, error-prone, and tightly coupled, since modifying one feature can lead to the risk of missing something important.

class Customer
    def update_email(new_email)
        # Update the customer's record
        @customer_email = new_email
        # Also update the email in the billing system
        BillingSystem.update_customer_email(self, new_email)
        # And update it in the notification system
        NotificationSystem.update_email(self, new_email)
    end
end

Instead of having to make email changes in multiple places, centralize the logic in one responsible class:

class EmailUpdater
    def initialize(customer)
        @customer = customer
    end

    def update_email(new_email)
        @customer.email = new_email
        notify_billing_system
        notify_notification_system
    end

    private

    def notify_billing_system
        BillingSystem.update_customer_email(@customer, @customer.email)
    end

    def notify_notification_system
        NotificationSystem.update_email(@customer, @customer.email)
    end
end

Parallel Inheritance Hierarchies arise when every time you create a subclass in one hierarchy, you’re forced to create a corresponding subclass in another hierarchy. This coupling between two (or more) class hierarchies leads to complexity and bloated code because changes in one hierarchy require changes in the other(s), making it harder to maintain and extend.
Let’s say you’re working on an application that models different types of vehicles and the tools needed to repair them. You start with the following class structure:

    class Vehicle
        def repair(tool)
            raise NotImplementedError
        end
    end

    class Car < Vehicle
        def repair(tool)
            tool.fix_car(self)
        end
    end

    class Bike < Vehicle
        def repair(tool)
            tool.fix_bike(self)
        end
    end

    class RepairTool
        def fix_car(car)
            raise NotImplementedError
        end

        def fix_bike(bike)
            raise NotImplementedError
        end
    end

    class CarRepairTool < RepairTool
        def fix_car(car)
            # Car-specific repair logic
        end
    end

    class BikeRepairTool < RepairTool
        def fix_bike(bike)
            # Bike-specific repair logic
        end
    end

The solution is to decouple the hierarchies, often using composition instead of inheritance. In this case, you could introduce an abstraction where the vehicle knows how to be repaired, and the tools don’t need to be subclassed for each type of vehicle.

    class Vehicle
        def repair(tool)
            tool.use_on(self)
        end
    end

    class Car < Vehicle
        def repair(tool)
            super(self)
        end
    end

    class Bike < Vehicle
        def repair(tool)
            super(self)
        end
    end

    class RepairTool
        def use_on(vehicle)
            vehicle_specific_repair(vehicle)
        end

    private

        def vehicle_specific_repair(vehicle)
            case vehicle
            when Car
                fix_car(vehicle)
            when Bike
                fix_bike(vehicle)
            else
                raise "Unknown vehicle type"
            end
        end

        def fix_car(car)
            # Car-specific repair logic
        end

        def fix_bike(bike)
            # Bike-specific repair logic
        end
    end

By addressing Change Preventers, you can make your code more flexible and easier to modify, reducing the risk of unintended consequences when introducing new features or changes.

Dispensables#

Lazy Class
Data Class
Speculative Generality
Duplicated Code

These are the segments that are pretty useless and should not be there at all.

Lazy Class: This is a class that does too little to justify its existence. It might have been created with the intention of doing more, but over time it becomes unnecessary, holding only one or two methods. Rather than letting these tiny classes hang around like useless baggage, you should merge their functionality into a more relevant class. The key idea here is to reduce unnecessary abstraction.
```
class CarPrinter
  def print(car)
    puts "Car: #{car.name}"
  end
end
```
In this example, CarPrinter is too small and could be integrated into another class that handles cars.
Data Class: These classes only hold fields and lack behavior. They are essentially glorified data containers, often created just to group data together. While sometimes useful, they can often be refactored to have more meaningful methods that manipulate the data they hold, giving them behavior. If they aren’t adding value through behavior, it’s better to rethink their necessity.
```
class Car
  attr_accessor :name, :model, :year
end
```
Instead of having a simple data class, you can add methods to handle the logic:
```
class Car
  attr_accessor :name, :model, :year

  def age
    Time.now.year - year
  end
end
```
Speculative Generality: This is code that is written with the thought, “We might need this someday.” Developers sometimes add extra layers of abstraction or functionality, assuming future requirements. However, this “just-in-case” code usually never gets used and only adds unnecessary complexity. It’s better to focus on what’s needed now and refactor later if future needs arise.
```
class VehicleFactory
  def create_vehicle(type)
    case type
    when 'car'
      Car.new
    when 'bike'
      Bike.new
    # Truck is speculative generality as there's no requirement for it yet
    when 'truck'
      Truck.new
    else
      raise "Unknown vehicle type"
    end
  end
end
```
In this case, you might have added support for trucks, even though you don’t need them yet. It’s better to remove the Truck logic until there’s an actual requirement.
Duplicated Code: When you see the same code repeated in multiple places, it’s a sign of a dispensable smell. Duplication makes the code harder to maintain because a change in one place requires you to remember to make the same change everywhere else. Refactor duplicated code into methods, classes, or modules to centralize the logic and reduce redundancy.

By eliminating Dispensables, you can keep your codebase lean and more focused on the actual functionality rather than unnecessary complexity.

Couplers#

Feature Envy
Inappropriate Intimacy
Message Chains
Middle Man

Feature Envy occurs when one class is excessively interested in the details of another class. It usually happens when methods in one class need to frequently access the data or methods of another class. This indicates that the methods might be better off in the class they are interacting with, or that the class in question has taken on too many responsibilities.

class Employee
    attr_accessor :name, :salary, :years_of_service

    def initialize(name, salary, years_of_service)
        @name = name
        @salary = salary
        @years_of_service = years_of_service
    end

    def bonus
        base_bonus = @salary * 0.10
        extra_bonus = @years_of_service * 100
        base_bonus + extra_bonus
    end
end

class Payroll
    def calculate_total_bonus(employees)
        total_bonus = 0
        employees.each do |employee|
        total_bonus += employee.bonus
        end
        total_bonus
    end
 end

In this example, Order has feature envy of Item, as it is accessing item details to calculate the total. You might consider moving the total calculation logic to the Item class or creating a dedicated OrderTotalCalculator.

class Employee
    attr_accessor :name, :salary, :years_of_service

    def initialize(name, salary, years_of_service)
        @name = name
        @salary = salary
        @years_of_service = years_of_service
    end

    def bonus
        base_bonus = @salary * 0.10
        extra_bonus = @years_of_service * 100
        base_bonus + extra_bonus
    end
end

class BonusCalculator
    def initialize(employees)
        @employees = employees
    end

    def total_bonus
        @employees.sum(&:bonus)
    end
end

class Payroll
    def calculate_total_bonus(employees)
        calculator = BonusCalculator.new(employees)
        calculator.total_bonus
    end
end

Inappropriate Intimacy happens when classes are too tightly coupled and have excessive knowledge of each other’s internal details. This can lead to classes becoming too interdependent and difficult to modify. Ideally, classes should interact with each other through well-defined interfaces and encapsulate their internal workings.

class Customer
  def update_address(new_address)
    address.street = new_address.street
    address.city = new_address.city
    # Direct manipulation of address internals
  end

  private

  def address
    @address ||= Address.new
  end
end

class Address
  attr_accessor :street, :city
end

Here, Customer is too intimately involved with Address. Instead, you could provide methods on Address for updating address details:

class Customer
  def update_address(new_address)
    address.update(new_address)
  end

  private

  def address
    @address ||= Address.new
  end
end

class Address
  def update(new_address)
    self.street = new_address.street
    self.city = new_address.city
  end
end

Message Chains occur when a series of method calls are made to get the desired result. It can lead to hard-to-read code and make it difficult to understand what’s going on, as it resembles a long chain of responsibility.
```
def print_invoice
  customer.orders.first.items.last.print_details
end
```
In this example, the print_invoice method involves a long chain of method calls. To refactor this, you could encapsulate the logic in a method or class:
```
class InvoicePrinter
  def initialize(customer)
    @customer = customer
  end

  def print
    order = @customer.orders.first
    item = order.items.last
    item.print_details
  end
end
```

Middle Man occurs when a class is simply passing requests to another class without adding any value. The middleman class doesn’t provide any additional behavior and just forwards calls, which can make the codebase unnecessarily complex and bloated.

class OrderManager
  def initialize(order)
    @order = order
  end

  def process_order
    @order.process
  end

  def cancel_order
    @order.cancel
  end
end

class Order
  def process
    # processing logic
  end

  def cancel
    # cancellation logic
  end
end

Here, OrderManager is acting as a middleman. Instead, you might interact directly with the Order class or use other design patterns that better suit the requirements:

class Order
  def process
    # processing logic
  end

  def cancel
    # cancellation logic
  end
end

By addressing Couplers, you can reduce tight coupling between classes, improve encapsulation, and create a more modular and maintainable codebase.

Code Smells Dilemma#

It’s the symptomatic nature of code smells that makes dealing with them complicated. They aren’t bugs or code errors — a specific issue with a clear fix. Code smells are a symptom that suggests low quality code. They can mean that there’s a larger problem, but sometimes they don’t. Coding is nuanced, and there are times when imperfect or “bad” code is necessary in a specific scenario.

For this reason, it can’t be assumed that a code smell is a sure sign of a problem — just that it’s a likely one. But for all that, they can’t simply be ignored. Code smells have to be investigated and confirmed before they can be eliminated.

This, of course, doesn’t always (or even often) happen. There are plenty of scenarios where busy developers need to work at a rapid pace with little time to investigate code smells they encounter along the way.

When left untreated, however, code smell can eventually become code rot, where code degrades over time causing the initial problem to grow and grow. Code rot increases technical debt and inevitably impedes the development process, often slowing velocity or creating roadblocks to successful deployment of new updates.

Thats all in this one folks.. let me know where i messed up in writing this :)