Code Smells

Overview

Code smells represent surface-level indicators that hint at deeper problems in code structure, design, or implementation. Martin Fowler popularized the term in his refactoring work, describing smells as symptoms that signal potential maintenance difficulties, design weaknesses, or technical debt accumulation. Unlike bugs or errors that cause immediate failures, code smells indicate areas where code works but exhibits characteristics that increase maintenance costs, reduce readability, or limit extensibility.

The concept originates from the observation that experienced developers develop intuition for problematic code patterns. Rather than requiring formal proof of a problem, a smell triggers investigation into whether refactoring would improve the codebase. Not all smells require immediate action, but recognizing them enables informed decisions about technical debt and code quality.

Code smells exist at multiple levels of abstraction. Method-level smells affect individual functions or procedures. Class-level smells involve design issues within object definitions. Architecture-level smells indicate problems in how components interact or how responsibilities distribute across the system. Each level requires different refactoring approaches and carries different risk-benefit profiles for remediation.

# Example of Long Method smell
def process_order(order)
  # Validate order
  return false unless order.items.any?
  return false unless order.customer.valid?
  return false if order.total < 0
  
  # Calculate totals
  subtotal = order.items.sum(&:price)
  tax = subtotal * 0.08
  shipping = calculate_shipping(order)
  total = subtotal + tax + shipping
  
  # Apply discounts
  if order.customer.premium?
    total *= 0.9
  end
  if order.total > 100
    shipping = 0
  end
  
  # Process payment
  payment = Payment.new(amount: total)
  return false unless payment.process
  
  # Update inventory
  order.items.each do |item|
    item.decrement_stock
  end
  
  # Send notifications
  OrderMailer.confirmation(order).deliver
  NotificationService.notify_warehouse(order)
  
  true
end

The distinction between a smell and a legitimate design choice depends on context. A large class might appropriately represent a complex domain concept, or it might indicate poor separation of concerns. A long parameter list might reflect genuine complexity in an operation, or it might suggest missing abstractions. Experience and domain knowledge inform these judgments.

Key Principles

Code smell identification relies on recognizing patterns that correlate with maintenance difficulties. These patterns manifest as violations of design principles, excessive complexity metrics, or structural characteristics that impede understanding. The presence of a smell does not automatically require refactoring, but triggers evaluation of whether the current structure serves long-term maintainability.

Smell Categories

Bloaters represent code elements that have grown excessively large. Long methods, large classes, primitive obsession, long parameter lists, and data clumps fall into this category. These smells emerge gradually as features accrue and abstractions fail to evolve with complexity. A class that starts with three responsibilities might accumulate seven or eight through incremental changes, each seeming reasonable in isolation.

Object-orientation abusers indicate misuse or absence of object-oriented principles. Switch statements that dispatch on type codes, refused bequests where subclasses ignore inherited behavior, temporary fields that remain unused most of the time, and alternative classes with different interfaces solving the same problem belong here. These smells often result from procedural thinking in object-oriented contexts.

Change preventers create friction during modification. Divergent change occurs when a single class requires modification for multiple different reasons. Shotgun surgery happens when a single change requires touching many classes. Parallel inheritance hierarchies force creation of paired subclasses. These smells directly increase the cost and risk of system evolution.

Dispensables represent elements that serve no clear purpose. Comments explaining what code does rather than why, duplicate code, lazy classes with insufficient responsibility, data classes with only getters and setters, and dead code create maintenance burden without providing value. Removing dispensables simplifies the system without losing functionality.

Couplers create excessive dependencies between elements. Feature envy occurs when methods access another object's data more than their own. Inappropriate intimacy develops when classes depend excessively on each other's internal details. Message chains create fragile dependencies on navigating object graphs. Middle men that only delegate without adding value introduce unnecessary indirection.

Smell Indicators

Quantitative metrics provide objective smell indicators. Method length measured in lines or statements correlates with comprehension difficulty. Cyclomatic complexity counting independent paths through code identifies decision-heavy logic. Class size measured by method count or total lines indicates separation of concerns issues. Depth of inheritance hierarchies and coupling between objects offer structural complexity measures.

Qualitative characteristics complement metrics. Difficulty explaining what a method or class does suggests unclear responsibilities. Trouble naming elements indicates missing or muddled abstractions. Fear of modifying code signals brittle design or insufficient test coverage. Frequent bugs in specific areas point to problematic structure.

Refactoring Connection

Each smell associates with specific refactoring techniques. Long methods decompose through extract method. Large classes split via extract class or extract subclass. Duplicate code consolidates through extract method or pull up method. Feature envy resolves via move method. Understanding smell-refactoring relationships enables targeted improvements.

The refactoring catalog provides mechanical steps for addressing smells. Automated refactoring tools implement these transformations with correctness guarantees. Manual refactoring requires comprehensive test coverage to verify behavior preservation. The safety and effort of refactoring influence whether addressing a smell makes economic sense.

Context Dependence

Smell evaluation requires context. A 200-line method in a bootstrapping script differs from a 200-line method in core business logic. A god class in a small utility differs from a god class coordinating major subsystems. Performance-critical code may accept smells that would be unacceptable elsewhere. Domain complexity sometimes manifests as code complexity without representing a design flaw.

Organizational factors influence smell tolerance. Codebases with comprehensive test suites allow more aggressive refactoring. Teams with continuous integration and deployment can incrementally improve code. Projects nearing end-of-life may not justify refactoring investments. Understanding these trade-offs prevents dogmatic application of smell detection.

Common Patterns

Long Method

Methods exceeding 20-30 lines often indicate multiple responsibilities or excessive detail at one abstraction level. Long methods hinder understanding because readers must track more state, follow more branches, and comprehend more operations simultaneously. The smell manifests through scrolling to see the entire method, difficulty summarizing what the method does, or trouble choosing a descriptive name.

# Long method with multiple concerns
def generate_report(user_id, start_date, end_date)
  user = User.find(user_id)
  orders = Order.where(user_id: user_id)
              .where('created_at >= ?', start_date)
              .where('created_at <= ?', end_date)
  
  total_revenue = 0
  total_items = 0
  categories = Hash.new(0)
  
  orders.each do |order|
    total_revenue += order.total
    order.items.each do |item|
      total_items += item.quantity
      categories[item.category] += item.quantity
    end
  end
  
  report = Report.new
  report.user_name = user.name
  report.date_range = "#{start_date} to #{end_date}"
  report.total_revenue = total_revenue
  report.total_items = total_items
  report.top_category = categories.max_by { |k, v| v }&.first
  
  report.save
  ReportMailer.send_report(user, report).deliver
  
  report
end

Extract method refactoring addresses long methods by identifying cohesive blocks and extracting them into well-named methods. Each extracted method operates at a consistent abstraction level and has a clear, single purpose. The original method becomes a sequence of intention-revealing method calls.

Large Class

Classes with dozens of methods, hundreds of lines, or numerous instance variables indicate multiple responsibilities. Large classes violate the single responsibility principle and become difficult to understand, test, and modify. Changes in one area risk breaking seemingly unrelated functionality. The class acts as a magnet attracting new features because it already does so much.

Extract class refactoring splits large classes by identifying cohesive subsets of data and behavior. Extract subclass creates specialized versions when variation exists. Replace conditional with polymorphism distributes type-specific behavior across classes. These techniques create focused classes with clear purposes.

Primitive Obsession

Overusing primitive types instead of small objects to represent domain concepts creates scattered logic and limits expressiveness. Representing money as floats, phone numbers as strings, or coordinates as separate x and y integers loses type safety and scatters validation and formatting logic.

# Primitive obsession
class Invoice
  attr_accessor :amount_cents, :currency_code
  
  def formatted_amount
    "#{currency_code} #{amount_cents / 100.0}"
  end
  
  def convert_to(target_currency)
    rate = CurrencyConverter.rate(currency_code, target_currency)
    self.amount_cents = (amount_cents * rate).round
    self.currency_code = target_currency
  end
end

# Replace with value object
class Money
  attr_reader :cents, :currency
  
  def initialize(cents, currency)
    @cents = cents
    @currency = currency
  end
  
  def to_s
    "#{currency} #{cents / 100.0}"
  end
  
  def convert_to(target_currency)
    rate = CurrencyConverter.rate(currency, target_currency)
    Money.new((cents * rate).round, target_currency)
  end
end

class Invoice
  attr_accessor :amount
  
  def formatted_amount
    amount.to_s
  end
  
  def convert_to(currency)
    self.amount = amount.convert_to(currency)
  end
end

Replace data value with object creates small classes representing domain concepts. These objects encapsulate validation, formatting, and related operations, centralizing logic that would otherwise scatter across the codebase.

Long Parameter List

Methods requiring many parameters become difficult to call correctly and signal missing abstractions. Parameters exceeding four or five create cognitive burden and increase the chance of passing arguments in wrong order or omitting required values.

# Long parameter list
def create_user(first_name, last_name, email, phone, address_line1, 
                address_line2, city, state, zip, country, newsletter_opt_in)
  # Implementation
end

# Introduce parameter object
class UserRegistration
  attr_accessor :first_name, :last_name, :email, :phone
  attr_accessor :address, :newsletter_opt_in
end

class Address
  attr_accessor :line1, :line2, :city, :state, :zip, :country
end

def create_user(registration)
  # Implementation with registration.first_name, registration.address.city, etc.
end

Introduce parameter object groups related parameters into a cohesive structure. Preserve whole object passes entire objects rather than extracting individual values. Replace parameter with method call eliminates parameters that receivers can determine themselves.

Data Clump

Groups of data items appearing together repeatedly suggest missing abstractions. Three or four values consistently passed as a unit or stored as separate fields indicate a concept that deserves its own class.

Duplicate Code

Identical or nearly identical code in multiple locations multiplies maintenance costs and creates consistency risks. Changes require updating multiple locations, and divergence creates subtle bugs. Duplicate code indicates missing abstractions or failed decomposition.

# Duplicate code in sibling classes
class CreditCardPayment
  def process
    validate_payment_method
    authorize_amount
    capture_funds
    log_transaction
    send_receipt
  end
end

class BankTransferPayment
  def process
    validate_payment_method
    authorize_amount
    capture_funds
    log_transaction
    send_receipt
  end
end

# Extract to template method
class Payment
  def process
    validate_payment_method
    authorize_amount
    capture_funds
    log_transaction
    send_receipt
  end
  
  def validate_payment_method
    raise NotImplementedError
  end
  
  def authorize_amount
    raise NotImplementedError
  end
  
  # Common implementations
  def log_transaction
    # Shared logging logic
  end
  
  def send_receipt
    # Shared receipt logic
  end
end

class CreditCardPayment < Payment
  def validate_payment_method
    # Credit card specific validation
  end
  
  def authorize_amount
    # Credit card authorization
  end
end

Extract method consolidates duplicate code into a single location. Pull up method moves common behavior to superclasses. Form template method creates abstract algorithms with specialized steps.

Switch Statements

Type-based conditionals that dispatch to different logic violate the open-closed principle and require modification when new types emerge. Switch statements on type codes or polymorphic type checking indicate missing polymorphism.

# Switch statement smell
class ShapeCalculator
  def area(shape)
    case shape.type
    when :circle
      Math::PI * shape.radius ** 2
    when :rectangle
      shape.width * shape.height
    when :triangle
      0.5 * shape.base * shape.height
    else
      raise "Unknown shape type"
    end
  end
end

# Replace with polymorphism
class Circle
  attr_reader :radius
  
  def area
    Math::PI * radius ** 2
  end
end

class Rectangle
  attr_reader :width, :height
  
  def area
    width * height
  end
end

class Triangle
  attr_reader :base, :height
  
  def area
    0.5 * base * height
  end
end

Replace conditional with polymorphism distributes type-specific behavior to classes. Replace type code with subclasses creates class hierarchies. Replace type code with state/strategy uses composition instead of inheritance.

Feature Envy

Methods that access another object's data more than their own suggest misplaced responsibility. The envious method belongs with the data it manipulates, improving cohesion and reducing coupling.

Move method transfers functionality to the appropriate class. Extract method can precede moving if the envious code mixes concerns. The refactored design places operations near their data, following the principle that data and behavior belong together.

Inappropriate Intimacy

Classes that excessively access each other's internals create tight coupling and make changes expensive. Private implementation details become public dependencies, preventing independent evolution.

Move method and move field reduce coupling by relocating functionality. Change bidirectional association to unidirectional eliminates unnecessary links. Replace inheritance with delegation loosens coupling when subclasses depend heavily on parent internals. Encapsulate collection protects internal collections from external manipulation.

Message Chains

Long chains of method calls traversing object graphs create fragile code that breaks when intermediate relationships change. Each link in the chain represents a dependency and exposes internal structure.

# Message chain smell
order.customer.address.city.zip_code

# Hide delegate
class Order
  def customer_zip_code
    customer.address.city.zip_code
  end
end

order.customer_zip_code

Hide delegate introduces intermediary methods that encapsulate navigation. Extract method can create meaningful queries that express intent rather than exposing structure.

Middle Man

Classes that only delegate to other classes without adding value create unnecessary indirection. Excessive delegation indicates that the middle man has outlived its purpose or never had one.

Remove middle man eliminates unnecessary delegators. Inline method incorporates trivial delegations. Replace delegation with inheritance makes relationships explicit when appropriate.

Speculative Generality

Code designed for future requirements that never materialize adds complexity without benefit. Abstract classes with one subclass, parameters never used, methods never called, and excessive indirection for current needs all exemplify speculative generality.

Collapse hierarchy removes unnecessary abstractions. Inline class eliminates pointless indirection. Remove parameter cleans up unused flexibility. Rename method makes intent clear when abstract names hide simple purposes.

Ruby Implementation

Ruby-Specific Smells

Ruby's dynamic nature enables certain smells that static languages prevent or manifest differently. Excessive metaprogramming obscures control flow and makes debugging difficult. Overusing method_missing creates implicit interfaces that break tooling and confuse developers. Abusing monkey patching modifies core classes in ways that cause conflicts and surprises.

# Excessive metaprogramming smell
class DynamicModel
  def method_missing(method_name, *args)
    if method_name.to_s.start_with?('find_by_')
      attribute = method_name.to_s.sub('find_by_', '')
      find_by_attribute(attribute, args.first)
    else
      super
    end
  end
  
  def respond_to_missing?(method_name, include_private = false)
    method_name.to_s.start_with?('find_by_') || super
  end
end

# Prefer explicit methods
class Model
  def self.find_by_name(name)
    find_by_attribute('name', name)
  end
  
  def self.find_by_email(email)
    find_by_attribute('email', email)
  end
end

Ruby blocks enable closure-based smells. Excessively long blocks mix abstraction levels. Blocks with side effects create action-at-a-distance problems. Nested blocks become difficult to follow.

# Long block smell
users.select do |user|
  user.active? && 
  user.subscribed? &&
  user.last_login > 30.days.ago &&
  user.posts.any? { |post| post.published? && post.views > 100 }
end.map do |user|
  {
    name: user.full_name,
    email: user.email,
    score: calculate_engagement_score(user),
    rank: determine_user_rank(user)
  }
end.sort_by { |data| -data[:score] }

# Extract methods for clarity
def active_engaged_users
  users.select { |user| active_and_engaged?(user) }
end

def active_and_engaged?(user)
  user.active? && 
  user.subscribed? &&
  recent_activity?(user) &&
  has_popular_posts?(user)
end

def user_summaries(users)
  users.map { |user| user_summary(user) }
       .sort_by { |data| -data[:score] }
end

Ruby Idiom Violations

Not following Ruby conventions creates code that feels foreign to Ruby developers. Using explicit returns everywhere, parentheses for all method calls, or Java-style getters and setters violates Ruby style.

# Non-idiomatic Ruby
class User
  def getName()
    return @name
  end
  
  def setName(name)
    @name = name
    return nil
  end
  
  def isActive()
    if @active == true
      return true
    else
      return false
    end
  end
end

# Idiomatic Ruby
class User
  attr_accessor :name
  
  def active?
    @active
  end
end

Not using Ruby's enumerable methods creates imperative code where declarative code would be clearer. Manual array iteration with index tracking, accumulator initialization, and explicit loop management smell when map, select, or reduce express intent more clearly.

# Imperative style smell
result = []
index = 0
while index < items.length
  item = items[index]
  if item.price > 10
    result << item.name.upcase
  end
  index += 1
end

# Declarative Ruby style
result = items.select { |item| item.price > 10 }
              .map { |item| item.name.upcase }

Rails-Specific Smells

Rails applications exhibit framework-specific smells. Fat controllers containing business logic violate MVC separation. Callbacks proliferating in models create implicit dependencies and make testing difficult. Direct database queries scattered throughout views tightly couple presentation to data access.

# Fat controller smell
class OrdersController < ApplicationController
  def create
    @order = Order.new(order_params)
    @order.user = current_user
    
    # Business logic in controller
    subtotal = @order.items.sum(&:price)
    tax = subtotal * 0.08
    @order.total = subtotal + tax
    
    if @order.total > 100
      @order.discount = @order.total * 0.1
      @order.total -= @order.discount
    end
    
    if @order.save
      @order.items.each { |item| item.decrement_stock }
      OrderMailer.confirmation(@order).deliver_later
      NotificationService.notify_warehouse(@order)
      redirect_to @order
    else
      render :new
    end
  end
end

# Extract to service object
class OrdersController < ApplicationController
  def create
    result = CreateOrder.new(current_user, order_params).call
    
    if result.success?
      redirect_to result.order
    else
      @order = result.order
      render :new
    end
  end
end

class CreateOrder
  def initialize(user, params)
    @user = user
    @params = params
  end
  
  def call
    order = build_order
    
    if order.save
      process_successful_order(order)
      Result.success(order)
    else
      Result.failure(order)
    end
  end
  
  private
  
  def build_order
    Order.new(@params).tap do |order|
      order.user = @user
      order.calculate_total
      order.apply_discounts
    end
  end
  
  def process_successful_order(order)
    order.items.each(&:decrement_stock)
    OrderMailer.confirmation(order).deliver_later
    NotificationService.notify_warehouse(order)
  end
end

Excessive ActiveRecord callbacks create implicit ordering dependencies. Modifications trigger chains of callbacks that become difficult to reason about. Testing requires triggering callbacks or mocking them, both problematic.

Module Misuse

Using modules solely for namespace organization when inheritance makes sense, or using inheritance when composition with modules would be more appropriate, creates awkward designs. Ruby's mixin system allows flexible composition, but overuse creates tangled dependencies.

# Module namespace smell when inheritance appropriate
module AnimalBehaviors
  class Eating
    def eat
      puts "Eating"
    end
  end
  
  class Sleeping
    def sleep
      puts "Sleeping"
    end
  end
end

class Dog
  def initialize
    @eating = AnimalBehaviors::Eating.new
    @sleeping = AnimalBehaviors::Sleeping.new
  end
  
  def eat
    @eating.eat
  end
  
  def sleep
    @sleeping.sleep
  end
end

# Better approach with mixins
module Eating
  def eat
    puts "Eating"
  end
end

module Sleeping
  def sleep
    puts "Sleeping"
  end
end

class Dog
  include Eating
  include Sleeping
end

Practical Examples

Refactoring God Class

Consider a User class that handles authentication, profile management, notifications, billing, and preferences.

# God class smell
class User
  attr_accessor :email, :password_hash, :name, :bio, :avatar_url
  attr_accessor :notification_preferences, :email_verified
  attr_accessor :subscription_plan, :payment_method, :billing_address
  
  def authenticate(password)
    BCrypt::Password.new(password_hash) == password
  end
  
  def update_password(old_password, new_password)
    return false unless authenticate(old_password)
    self.password_hash = BCrypt::Password.create(new_password)
    save
  end
  
  def send_notification(type, message)
    return unless notification_preferences[type]
    
    case type
    when :email
      UserMailer.notification(self, message).deliver
    when :sms
      SmsService.send(phone, message)
    when :push
      PushService.send(device_tokens, message)
    end
  end
  
  def process_subscription_payment
    return unless subscription_plan.present?
    
    charge = Stripe::Charge.create(
      amount: subscription_plan.price_cents,
      currency: 'usd',
      customer: stripe_customer_id,
      description: "Subscription: #{subscription_plan.name}"
    )
    
    create_payment_record(charge)
    extend_subscription
  end
  
  def update_profile(attributes)
    self.name = attributes[:name] if attributes[:name]
    self.bio = attributes[:bio] if attributes[:bio]
    
    if attributes[:avatar]
      self.avatar_url = upload_avatar(attributes[:avatar])
    end
    
    save
  end
  
  # Many more methods...
end

Refactor by extracting cohesive responsibilities into focused classes:

class User
  attr_accessor :email, :name
  
  def authenticator
    @authenticator ||= UserAuthenticator.new(self)
  end
  
  def profile
    @profile ||= UserProfile.new(self)
  end
  
  def notifier
    @notifier ||= UserNotifier.new(self)
  end
  
  def subscription
    @subscription ||= UserSubscription.new(self)
  end
end

class UserAuthenticator
  def initialize(user)
    @user = user
  end
  
  def authenticate(password)
    BCrypt::Password.new(@user.password_hash) == password
  end
  
  def update_password(old_password, new_password)
    return false unless authenticate(old_password)
    @user.password_hash = BCrypt::Password.create(new_password)
    @user.save
  end
end

class UserProfile
  def initialize(user)
    @user = user
  end
  
  def update(attributes)
    @user.name = attributes[:name] if attributes[:name]
    @user.bio = attributes[:bio] if attributes[:bio]
    upload_avatar(attributes[:avatar]) if attributes[:avatar]
    @user.save
  end
  
  private
  
  def upload_avatar(file)
    uploader = AvatarUploader.new(@user)
    uploader.store!(file)
    @user.avatar_url = uploader.url
  end
end

class UserNotifier
  def initialize(user)
    @user = user
    @preferences = user.notification_preferences
  end
  
  def notify(type, message)
    return unless enabled?(type)
    channel_for(type).send(@user, message)
  end
  
  private
  
  def enabled?(type)
    @preferences[type]
  end
  
  def channel_for(type)
    {
      email: EmailNotificationChannel,
      sms: SmsNotificationChannel,
      push: PushNotificationChannel
    }[type]
  end
end

class UserSubscription
  def initialize(user)
    @user = user
  end
  
  def process_payment
    return unless active?
    
    charge = create_stripe_charge
    record_payment(charge)
    extend_period
  end
  
  private
  
  def active?
    @user.subscription_plan.present?
  end
  
  def create_stripe_charge
    Stripe::Charge.create(
      amount: @user.subscription_plan.price_cents,
      currency: 'usd',
      customer: @user.stripe_customer_id,
      description: charge_description
    )
  end
end

Eliminating Shotgun Surgery

When adding a new payment method requires changes across controllers, models, services, and views, shotgun surgery occurs.

# Changes scattered across multiple files

# app/controllers/checkout_controller.rb
def process_payment
  case params[:payment_type]
  when 'credit_card'
    process_credit_card
  when 'paypal'
    process_paypal
  when 'bitcoin'  # New case added
    process_bitcoin
  end
end

# app/models/order.rb
def payment_type_display
  case payment_type
  when 'credit_card'
    'Credit Card'
  when 'paypal'
    'PayPal'
  when 'bitcoin'  # New case added
    'Bitcoin'
  end
end

# app/services/payment_processor.rb
def process(order)
  case order.payment_type
  when 'credit_card'
    CreditCardProcessor.process(order)
  when 'paypal'
    PayPalProcessor.process(order)
  when 'bitcoin'  # New case added
    BitcoinProcessor.process(order)
  end
end

# app/views/orders/show.html.erb
# <%= case @order.payment_type
     when 'credit_card' then 'CC Icon'
     when 'paypal' then 'PP Icon'
     when 'bitcoin' then 'BTC Icon'  # New case added
     end %>

Refactor to polymorphic design that isolates changes:

# Payment method registry pattern
class PaymentMethodRegistry
  def self.register(key, klass)
    registry[key] = klass
  end
  
  def self.find(key)
    registry[key] || raise("Unknown payment method: #{key}")
  end
  
  def self.all
    registry.values
  end
  
  private
  
  def self.registry
    @registry ||= {}
  end
end

class PaymentMethod
  def self.register_as(key)
    PaymentMethodRegistry.register(key, self)
  end
  
  def self.key
    raise NotImplementedError
  end
  
  def self.display_name
    raise NotImplementedError
  end
  
  def self.icon
    raise NotImplementedError
  end
  
  def process(order)
    raise NotImplementedError
  end
end

class CreditCardPayment < PaymentMethod
  register_as :credit_card
  
  def self.display_name
    'Credit Card'
  end
  
  def self.icon
    'credit-card'
  end
  
  def process(order)
    # Credit card processing
  end
end

class PayPalPayment < PaymentMethod
  register_as :paypal
  
  def self.display_name
    'PayPal'
  end
  
  def self.icon
    'paypal'
  end
  
  def process(order)
    # PayPal processing
  end
end

# Adding Bitcoin requires only one new file
class BitcoinPayment < PaymentMethod
  register_as :bitcoin
  
  def self.display_name
    'Bitcoin'
  end
  
  def self.icon
    'bitcoin'
  end
  
  def process(order)
    # Bitcoin processing
  end
end

# Controller simplified
class CheckoutController
  def process_payment
    payment_method = PaymentMethodRegistry.find(params[:payment_type])
    processor = payment_method.new
    processor.process(@order)
  end
end

# Views access metadata
<%= PaymentMethodRegistry.all.map do |method|
  link_to method.display_name, '#', data: { icon: method.icon }
end %>

Removing Data Clumps

Three coordinates (x, y, z) appearing together throughout a physics simulation indicate a missing vector abstraction.

# Data clump smell
class Particle
  attr_accessor :pos_x, :pos_y, :pos_z
  attr_accessor :vel_x, :vel_y, :vel_z
  attr_accessor :acc_x, :acc_y, :acc_z
  
  def update(delta_time)
    vel_x += acc_x * delta_time
    vel_y += acc_y * delta_time
    vel_z += acc_z * delta_time
    
    pos_x += vel_x * delta_time
    pos_y += vel_y * delta_time
    pos_z += vel_z * delta_time
  end
  
  def distance_to(other)
    dx = pos_x - other.pos_x
    dy = pos_y - other.pos_y
    dz = pos_z - other.pos_z
    Math.sqrt(dx*dx + dy*dy + dz*dz)
  end
end

def attract(p1, p2, force)
  dx = p2.pos_x - p1.pos_x
  dy = p2.pos_y - p1.pos_y
  dz = p2.pos_z - p1.pos_z
  distance = Math.sqrt(dx*dx + dy*dy + dz*dz)
  
  p1.acc_x += force * dx / distance
  p1.acc_y += force * dy / distance
  p1.acc_z += force * dz / distance
end

Extract vector class to encapsulate coordinate operations:

class Vector3
  attr_reader :x, :y, :z
  
  def initialize(x, y, z)
    @x, @y, @z = x, y, z
  end
  
  def +(other)
    Vector3.new(x + other.x, y + other.y, z + other.z)
  end
  
  def -(other)
    Vector3.new(x - other.x, y - other.y, z - other.z)
  end
  
  def *(scalar)
    Vector3.new(x * scalar, y * scalar, z * scalar)
  end
  
  def magnitude
    Math.sqrt(x*x + y*y + z*z)
  end
  
  def normalize
    mag = magnitude
    Vector3.new(x / mag, y / mag, z / mag)
  end
end

class Particle
  attr_accessor :position, :velocity, :acceleration
  
  def initialize
    @position = Vector3.new(0, 0, 0)
    @velocity = Vector3.new(0, 0, 0)
    @acceleration = Vector3.new(0, 0, 0)
  end
  
  def update(delta_time)
    @velocity = velocity + (acceleration * delta_time)
    @position = position + (velocity * delta_time)
  end
  
  def distance_to(other)
    (position - other.position).magnitude
  end
end

def attract(p1, p2, force)
  direction = (p2.position - p1.position).normalize
  p1.acceleration = p1.acceleration + (direction * force)
end

Design Considerations

When to Refactor

Not all smells require immediate attention. Refactoring requires time, introduces risk, and may not provide commensurate benefit. Decision factors include change frequency in the affected code, team familiarity with the area, test coverage quality, and remaining project lifetime.

Code changed frequently justifies refactoring investment. Each modification becomes easier, bugs decrease, and new features integrate more smoothly. Code rarely touched may not warrant refactoring regardless of smell intensity. The return on refactoring investment depends on future interaction frequency.

# Low-change-frequency smell
class LegacyImporter
  # 500-line method importing data from deprecated system
  # Used once annually during audit
  # Works correctly despite smell
end

# High-change-frequency smell  
class OrderProcessor
  # 200-line method processing orders
  # Modified every sprint for new features
  # Frequent bugs due to complexity
  # Justifies refactoring investment
end

Team capability influences refactoring decisions. Teams experienced with refactoring patterns and supported by comprehensive test suites can safely improve code incrementally. Teams lacking these capabilities risk introducing bugs or spending excessive time. Building refactoring capability itself becomes a prerequisite investment.

Technical Debt Management

Smells represent technical debt that accrues interest through increased maintenance costs. Unlike financial debt, technical debt lacks precise quantification. Teams estimate debt impact through relative difficulty metrics, bug rates, or velocity changes in affected areas.

Intentionally accepting debt makes sense under deadline pressure or with prototype code. The critical factor is conscious decision-making and tracking. Accidental debt accumulation from lack of attention creates more severe problems than deliberate debt taken for business reasons.

# Documented technical debt
class QuickAndDirtyReport
  # TODO: Technical debt - Extract classes, add tests
  # Reason: Critical deadline for investor meeting
  # Impact: Hard to modify, no test coverage
  # Remediation: Allocated Q2 2025 sprint
  # Tracking: TECH-1234
  
  def generate
    # 300 lines of procedural code
  end
end

Debt repayment strategies include incremental refactoring during feature work, dedicated refactoring sprints, and boy scout rule (leave code better than found). Incremental refactoring spreads cost across feature development. Dedicated sprints tackle systemic issues but require business justification. The boy scout rule creates gradual improvement without explicit allocation.

Performance vs Cleanliness Trade-offs

Performance optimization often conflicts with clean design. Inlining methods eliminates call overhead but increases duplication. Caching breaks encapsulation. Custom memory management adds complexity. These trade-offs require careful evaluation of actual performance requirements versus assumed needs.

Measure before optimizing. Profiling identifies genuine bottlenecks worth performance-smell trade-offs. Premature optimization creates unnecessary complexity. Many smells have negligible performance impact in non-critical code paths. Reserve performance compromises for proven hotspots with meaningful impact.

# Clean but potentially slow
class DataProcessor
  def process(items)
    items.select { |item| valid?(item) }
         .map { |item| transform(item) }
         .each { |item| store(item) }
  end
  
  def valid?(item)
    item.status == 'active' && item.value > 0
  end
  
  def transform(item)
    Item.new(name: item.name.upcase, value: item.value * 2)
  end
end

# Optimized for large datasets but more complex
class DataProcessor
  def process(items)
    result = Array.new(items.size)
    write_index = 0
    
    items.each do |item|
      next unless item.status == 'active' && item.value > 0
      
      result[write_index] = Item.new(
        name: item.name.upcase,
        value: item.value * 2
      )
      write_index += 1
      
      store(result[write_index - 1])
    end
    
    result[0...write_index]
  end
end

Domain complexity sometimes manifests as code complexity without representing a design flaw. Insurance underwriting, tax calculations, or scientific simulations contain inherent complexity that cannot be simplified away. Distinguish essential complexity from accidental complexity introduced by poor design.

Context-Specific Tolerance

Different contexts demand different smell tolerance levels. Core business logic requires the highest standards. Edge cases, throwaway scripts, and exploratory prototypes accept more smells. This differentiation allows efficient resource allocation while maintaining quality where it matters most.

# Core business logic - low smell tolerance
class InvoiceCalculator
  def calculate(line_items, customer)
    subtotal = Subtotal.new(line_items)
    tax = TaxCalculator.new(customer.tax_jurisdiction)
    discount = DiscountPolicy.new(customer).calculate(subtotal)
    
    Invoice.new(
      subtotal: subtotal,
      tax: tax.calculate(subtotal - discount),
      discount: discount
    )
  end
end

# One-off data migration script - higher smell tolerance
# db/migrate/20250315_migrate_legacy_data.rb
class MigrateLegacyData < ActiveRecord::Migration[7.0]
  def up
    # 150-line procedural script
    # Runs once, throws away
    # Smell acceptable given context
  end
end

Legacy integration layers dealing with external constraints may require smells to accommodate external design. Wrapping poorly designed APIs, integrating with legacy systems, or interfacing with third-party code that violates good design sometimes necessitates local smells to isolate external problems.

Tools & Ecosystem

RuboCop

RuboCop enforces Ruby style guide and detects many code smells automatically. Configured through rubocop.yml, it checks method length, cyclomatic complexity, class size, parameter counts, and hundreds of other metrics. RuboCop supports custom cops for project-specific standards.

# .rubocop.yml
Metrics/MethodLength:
  Max: 25
  Exclude:
    - 'db/migrate/**/*'

Metrics/ClassLength:
  Max: 200
  CountComments: false

Metrics/CyclomaticComplexity:
  Max: 10

Metrics/PerceivedComplexity:
  Max: 10

Metrics/AbcSize:
  Max: 20

Layout/LineLength:
  Max: 120

RuboCop auto-correction fixes many issues automatically. Safe corrections apply with --auto-correct flag. Unsafe corrections requiring human review use --auto-correct-all. Integration with editors provides real-time feedback during development.

# Run RuboCop on entire project
rubocop

# Auto-correct safe issues
rubocop --auto-correct

# Check specific files
rubocop app/models/user.rb

# Generate todo file for gradual adoption
rubocop --auto-gen-config

Reek

Reek specifically targets code smells with dedicated detectors. It identifies feature envy, data clumps, long parameter lists, utility functions, and other object-oriented design issues. Reek's strength lies in detecting design problems beyond style issues.

# .reek.yml
detectors:
  LongParameterList:
    max_params: 4
    
  TooManyStatements:
    max_statements: 10
    
  UtilityFunction:
    enabled: true
    
  DataClump:
    max_copies: 2
    min_clump_size: 3
    
  FeatureEnvy:
    enabled: true

Reek integration with CI pipelines prevents new smells from entering the codebase. Configure smell thresholds and fail builds exceeding limits. Gradual improvement strategies use todo files tracking accepted smells with remediation plans.

Flog

Flog calculates complexity scores for methods based on abstract syntax tree analysis. Higher scores indicate more complex code requiring attention. Unlike line-count metrics, Flog accounts for branching, nesting, and method calls.

# Analyze complexity
flog app/models/

# Show worst methods
flog --all app/models/ | sort -rn | head -20

# Generate complexity report
flog --score app/models/ > complexity_report.txt

Complexity trends over time provide objective technical debt metrics. Track Flog scores for critical classes and watch for increases. Set thresholds triggering refactoring discussions when exceeded.

Flay

Flay detects structural code duplication beyond simple text matching. It identifies similar code structures with different variable names or minor variations. This catches duplicated logic that text-based tools miss.

# Find structural duplication
flay app/

# Show duplicated code
flay --diff app/

# Minimum mass threshold
flay --mass 25 app/

Code Climate

Code Climate provides commercial static analysis combining multiple engines. It aggregates RuboCop, Reek, and other tools into unified quality metrics. Code Climate tracks technical debt, complexity trends, and test coverage over time.

Integration with GitHub pull requests provides automated code review feedback. Maintainability ratings quantify code quality changes. Technical debt estimates translate complexity into time costs.

Rubycritic

Rubycritic combines multiple analysis tools into comprehensive quality reports. It generates HTML reports showing smells, complexity metrics, churn analysis, and test coverage together. The unified view helps prioritize refactoring efforts.

# Generate quality report
rubycritic app/

# Compare branches
rubycritic --mode-ci --branch main

# Minimum score threshold
rubycritic --minimum-score 90 app/

Churn analysis combines complexity with change frequency. High-complexity, high-churn files represent the most problematic technical debt. Focus refactoring efforts on these files for maximum impact.

Continuous Integration

CI integration makes smell detection part of the development workflow. Configure thresholds and fail builds exceeding acceptable limits. Balance strictness with practicality to avoid causing friction.

# .github/workflows/quality.yml
name: Code Quality

on: [pull_request]

jobs:
  quality:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      
      - name: Set up Ruby
        uses: ruby/setup-ruby@v1
        with:
          bundler-cache: true
      
      - name: Run RuboCop
        run: bundle exec rubocop
      
      - name: Run Reek
        run: bundle exec reek
      
      - name: Run Flog
        run: bundle exec flog --threshold 20 app/

Editor Integration

Real-time feedback during development prevents smells from entering the codebase. Configure editors to run RuboCop and Reek on save. Inline warnings make smell detection part of the coding process rather than a post-hoc review.

VSCode, RubyMine, and Vim support Ruby linting plugins. Language server protocol implementations provide consistent experience across editors. Configure severity levels to distinguish between errors and suggestions.

Reference

Smell Categories

Common Smells Quick Reference

Ruby-Specific Smells

Refactoring Techniques

Complexity Metrics

Tool Configuration Priorities