Code smells that suggest a pattern: a step-by-step guide

Code smells that suggest a pattern

After nine decades of building production systems, I've learned that the worst code isn't always malicious—it's often just incomplete. Code smells that suggest a pattern are those warning signs in your codebase telling you that a proven solution exists, waiting to be applied. When you recognize these symptoms early, you can refactor deliberately instead of letting technical debt compound.

In this article, I'll walk through four common smells I encounter in production platforms and the design patterns that cure them. Each one comes from real scenarios: growing switch statements in payment processors, monster constructors in ORM entities, defensive null checking throughout business logic, and state flags that contradict each other in concurrent systems.

Growing switch statements → Strategy pattern

You start with two payment methods: card and bank transfer. A simple switch statement handles it. Six months later, you add crypto, buy-now-pay-later, digital wallets, and regional payment gateways. The switch statement now spans fifty lines, each branch responsible for vastly different logic. Every new payment method forces you back into that one file. This is the classic smell.

The Strategy pattern treats each payment method as an interchangeable algorithm. Instead of a switch statement deciding behavior, you define a common interface and let each strategy implement it independently.

interface PaymentStrategy {
  validate(amount: number, details: PaymentDetails): Promise<boolean>;
  process(amount: number, details: PaymentDetails): Promise<PaymentResult>;
  refund(transactionId: string, amount: number): Promise<void>;
}

class CardPaymentStrategy implements PaymentStrategy {
  async validate(amount: number, details: PaymentDetails): Promise<boolean> {
    return details.cardNumber?.length === 16;
  }

  async process(amount: number, details: PaymentDetails): Promise<PaymentResult> {
    // Stripe integration
    return { transactionId: '...', status: 'completed' };
  }

  async refund(transactionId: string, amount: number): Promise<void> {
    // Stripe refund logic
  }
}

class CryptoPaymentStrategy implements PaymentStrategy {
  async validate(amount: number, details: PaymentDetails): Promise<boolean> {
    return details.walletAddress?.startsWith('0x');
  }

  async process(amount: number, details: PaymentDetails): Promise<PaymentResult> {
    // Web3 transaction logic
    return { transactionId: '...', status: 'pending' };
  }

  async refund(transactionId: string, amount: number): Promise<void> {
    // Blockchain refund logic
  }
}

class PaymentProcessor {
  constructor(private strategies: Map<string, PaymentStrategy>) {}

  async processPayment(
    method: string,
    amount: number,
    details: PaymentDetails
  ): Promise<PaymentResult> {
    const strategy = this.strategies.get(method);
    if (!strategy) throw new Error(`Unknown payment method: ${method}`);

    const isValid = await strategy.validate(amount, details);
    if (!isValid) throw new Error('Validation failed');

    return strategy.process(amount, details);
  }
}

Now adding a new payment method means creating a new class, not touching existing code. The processor remains stable. I've used this pattern in every e-commerce platform I've built—from processing credit cards in Singapore to handling regional payment gateways in South Asian markets.

Complex constructors → Builder pattern

You have a User entity with twenty optional fields: email, phone, address, billing address, subscription tier, preferences, avatar URL, two-factor auth settings, and so on. You end up with constructors like new User(email, null, phone, address, null, true, false, ...) that no one can read. Worse, you add a new field and suddenly all the call sites break because they're relying on positional arguments.

The Builder pattern makes object construction explicit and readable. Each field gets its own setter-like method, and you call build() only when done.

class UserBuilder {
  private email: string = '';
  private phone?: string;
  private address?: Address;
  private billingAddress?: Address;
  private subscriptionTier: string = 'free';
  private twoFactorEnabled: boolean = false;
  private avatarUrl?: string;

  setEmail(email: string): UserBuilder {
    this.email = email;
    return this;
  }

  setPhone(phone: string): UserBuilder {
    this.phone = phone;
    return this;
  }

  setAddress(address: Address): UserBuilder {
    this.address = address;
    return this;
  }

  setBillingAddress(address: Address): UserBuilder {
    this.billingAddress = address;
    return this;
  }

  setSubscriptionTier(tier: string): UserBuilder {
    this.subscriptionTier = tier;
    return this;
  }

  enableTwoFactor(): UserBuilder {
    this.twoFactorEnabled = true;
    return this;
  }

  setAvatarUrl(url: string): UserBuilder {
    this.avatarUrl = url;
    return this;
  }

  build(): User {
    if (!this.email) throw new Error('Email is required');
    return new User(
      this.email,
      this.phone,
      this.address,
      this.billingAddress,
      this.subscriptionTier,
      this.twoFactorEnabled,
      this.avatarUrl
    );
  }
}

// Usage
const user = new UserBuilder()
  .setEmail('alice@example.com')
  .setPhone('+94771234567')
  .setSubscriptionTier('premium')
  .enableTwoFactor()
  .build();

This scales beautifully. Adding a new field means adding a new setter; old code keeps working. I've used builders extensively in CRM and trading platforms where domain objects have many optional configuration points. The fluent interface also makes intent clear at call sites.

Many if-null checks → Null Object pattern

Your ride-hailing service has a Discount system. Sometimes a ride qualifies for a discount, sometimes it doesn't. Throughout your pricing engine, you write:

if (discount !== null && discount.isActive()) {
  finalPrice = basePrice * (1 - discount.getPercentage());
} else {
  finalPrice = basePrice;
}

This pattern repeats everywhere: discount calculations, loyalty point multipliers, promotional flags. The code becomes brittle—if a single null check is missed, the system crashes in production.

The Null Object pattern eliminates these checks by providing a neutral object that implements the same interface but does nothing harmful.

interface Discount {
  isActive(): boolean;
  getPercentage(): number;
}

class ActiveDiscount implements Discount {
  constructor(private percentage: number) {}

  isActive(): boolean {
    return true;
  }

  getPercentage(): number {
    return this.percentage;
  }
}

class NoDiscount implements Discount {
  isActive(): boolean {
    return false;
  }

  getPercentage(): number {
    return 0;
  }
}

class PricingEngine {
  calculateFare(basePrice: number, discount: Discount): number {
    // No null check needed
    const discountRate = discount.isActive() ? discount.getPercentage() : 0;
    return basePrice * (1 - discountRate);
  }
}

// Usage
const activeDiscount: Discount = new ActiveDiscount(0.2);
const noDiscount: Discount = new NoDiscount();

engine.calculateFare(1000, activeDiscount); // Applies 20% discount
engine.calculateFare(1000, noDiscount);     // No discount

The beauty here is that your business logic doesn't change—you still check isActive(), but the null propagation stops. I've applied this across travel booking systems and SaaS platforms where optional features are common. The reduction in defensive code is substantial, and the intention becomes clearer.

Flags that conflict → State pattern

You're building an order processing system. Orders can be pending, confirmed, shipped, or cancelled. At first, you use boolean flags: isPending, isConfirmed, isShipped. But what happens when someone sets both isPending = true and isShipped = true? The system enters an invalid state. You add guards everywhere:

if (order.isPending && !order.isShipped && !order.isCancelled) {
  order.isConfirmed = true;
} else {
  throw new Error('Invalid state transition');
}

This becomes a maintenance nightmare in concurrent systems (like our ride-hailing platform with Kafka consumers). Two messages arrive out of order, and you've corrupted the order state.

The State pattern encapsulates each state as its own object, defining exactly which transitions are legal.

interface OrderState {
  confirm(order: Order): void;
  ship(order: Order): void;
  cancel(order: Order): void;
}

class PendingState implements OrderState {
  confirm(order: Order): void {
    order.setState(new ConfirmedState());
  }

  ship(order: Order): void {
    throw new Error('Cannot ship a pending order');
  }

  cancel(order: Order): void {
    order.setState(new CancelledState());
  }
}

class ConfirmedState implements OrderState {
  confirm(order: Order): void {
    throw new Error('Order already confirmed');
  }

  ship(order: Order): void {
    order.setState(new ShippedState());
  }

  cancel(order: Order): void {
    throw new Error('Cannot cancel a confirmed order');
  }
}

class ShippedState implements OrderState {
  confirm(order: Order): void {
    throw new Error('Cannot confirm a shipped order');
  }

  ship(order: Order): void {
    throw new Error('Order already shipped');
  }

  cancel(order: Order): void {
    throw new Error('Cannot cancel a shipped order');
  }
}

class CancelledState implements OrderState {
  confirm(order: Order): void {
    throw new Error('Cannot confirm a cancelled order');
  }

  ship(order: Order): void {
    throw new Error('Cannot ship a cancelled order');
  }

  cancel(order: Order): void {
    throw new Error('Order already cancelled');
  }
}

class Order {
  private state: OrderState = new PendingState();

  setState(state: OrderState): void {
    this.state = state;
  }

  confirm(): void {
    this.state.confirm(this);
  }

  ship(): void {
    this.state.ship(this);
  }

  cancel(): void {
    this.state.cancel(this);
  }
}

Now invalid transitions throw immediately. The state machine is self-documenting: looking at each state class tells you what transitions are allowed. When processing async events from Kafka, this prevents the entire class of race-condition bugs I've debugged in production.

Common mistakes

• Over-applying patterns prematurely: Don't refactor to Strategy when you have two payment methods. Wait for the third or fourth. Premature abstraction creates more cognitive load than it solves.
• Mixing patterns with frameworks: ORMs like TypeORM have their own entity patterns. Forcing Builder on top of decorators creates friction. Understand what your framework already provides.
• Forgetting the lifecycle cost: Patterns add indirection. State pattern means more objects in memory. Strategy means more files to navigate. Use patterns when the smell is strong enough to justify it.
• Null Object hiding real errors: If a discount is unexpectedly null in production, you want to know. A silent NoDiscount might mask a data bug. Logging or monitoring is still needed.
• State pattern without validation: Encapsulating transitions is good, but always validate the prerequisites. An order can't be confirmed if payment failed. The state doesn't know that.

Key takeaways

• Code smells that suggest a pattern are specific: growing switch statements, complex constructors, defensive null checks, and conflicting flags all have proven cures.
• Strategy eliminates branching logic by making algorithms interchangeable; each strategy is independently testable and deployable.
• Builder scales object construction by making it explicit and chainable; especially powerful for domain objects with many optional fields.
• Null Object removes defensive null checks throughout the codebase by providing a neutral implementation of the interface.
• State encapsulates valid transitions and prevents invalid state combinations, critical for concurrent systems and order processing pipelines.
• Don't apply patterns before the smell is strong enough; premature abstraction creates technical debt of its own.
• Patterns are tools, not rules. Understand the underlying problem (maintainability, testability, correctness) before reaching for the solution.