Sprout 클래스 패턴으로 루비 레거시 코드 리팩토링하기

레거시 애플리케이션을 유지보수하다 보면 가장 골치 아픈 문제 중 하나가 바로 코드가 테스트 가능성을 고려하지 않고 작성되어 있다는 점입니다. 그 결과 의미 있는 테스트를 작성하기 어렵거나, 심지어 불가능한 경우도 많습니다.

이건 전형적인 '닭과 달걀'의 문제입니다. 레거시 애플리케이션에 테스트를 추가하려면 먼저 코드를 변경해야 하는데, 정작 테스트가 없으면 코드를 안심하고 변경할 수 없기 때문입니다!

이런 역설은 어떻게 해결해야 할까요?

이 주제는 마이클 페더스(Michael Feathers)의 명저 『Working Effectively with Legacy Code』에서 여러 차례 다뤄집니다. 오늘은 이 책에서 소개하는 기법 중 하나인 Sprout Class(새싹 클래스)를 자세히 살펴보겠습니다.

레거시 코드 만나보기

오래된 ActiveRecord 클래스인 Appointment를 살펴보겠습니다. 이미 상당히 길지만, 실제 프로젝트에서는 이보다 수백 줄 더 길어지곤 합니다.

class Appointment < ActiveRecord::Base 
  has_many :appointment_services, :dependent => :destroy
  has_many :services, :through => :appointment_services
  has_many :appointment_products, :dependent => :destroy
  has_many :products, :through => :appointment_products
  has_many :payments, :dependent => :destroy
  has_many :transaction_items
  belongs_to :client
  belongs_to :stylist
  belongs_to :time_block_type

  def record_transactions
    transaction_items.destroy_all
    if paid_for?
      save_service_transaction_items
      save_product_transaction_items
      save_tip_transaction_item
    end
  end

  def save_service_transaction_items
    appointment_services.reload.each { |s| s.save_transaction_item(self.id) }
  end

  def save_product_transaction_items
    appointment_products.reload.each { |p| p.save_transaction_item(self.id) }
  end

  def save_tip_transaction_item
    TransactionItem.create!(
      :appointment_id => self.id,
      :stylist_id => self.stylist_id,
      :label => "Tip",
      :price => self.tip,
      :transaction_item_type_id => TransactionItemType.find_or_create_by_code("TIP").id
    )
  end
end

새로운 기능 추가하기

거래 내역 보고 기능에 새로운 기능을 추가해 달라는 요청을 받았다고 가정해 봅시다. 그런데 Appointment 클래스는 의존성이 너무 많아서 대규모 리팩토링 없이는 테스트할 수 없습니다. 이럴 때 어떻게 진행해야 할까요?

선택지 1: 일단 코드를 그냥 수정한다

def record_transactions
  transaction_items.destroy_all
  if paid_for?
    save_service_transaction_items
    save_product_transaction_items
    save_tip_transaction_item
    send_thank_you_email_to_client # New code
  end
end

def send_thank_you_email_to_client
  ThankYouMailer.thank_you_email(self).deliver
end

이 방식에는 문제가 있습니다

위 코드에는 두 가지 문제가 있습니다.

Appointment 클래스는 이미 여러 가지 책임을 떠안고 있습니다(단일 책임 원칙 위반). 그중 하나가 바로 거래 내역 기록입니다. 여기에 거래 관련 코드를 더 추가하면 코드는 점점 더 나빠질 뿐입니다.
새로운 통합 테스트(integration test)를 작성해서 이메일이 실제로 발송되는지 확인할 수는 있겠지만, Appointment 클래스 자체가 테스트 가능한 상태가 아니므로 단위 테스트는 추가할 수 없습니다. 결국 테스트되지 않은 코드를 계속 쌓아가는 셈인데, 이는 당연히 좋지 않습니다. 실제로 마이클 페더스는 레거시 코드를 "테스트가 없는 코드"라고 정의했습니다. 그렇다면 우리는 레거시 코드 위에 또 다른 레거시 코드를 얹고 있는 것이나 마찬가지입니다.

더 나은 방법: 책임을 분리하자

새 코드를 인라인으로 그냥 추가하는 것보다 나은 해결책은 거래 기록 로직을 별도의 클래스로 추출하는 것입니다. 이름을 TransactionRecorder라고 지어 보겠습니다:

class TransactionRecorder 
  def initialize(options)
    @appointment_id       = options[:appointment_id]
    @appointment_services = options[:appointment_services]
    @appointment_products = options[:appointment_products]
    @stylist_id           = options[:stylist_id]
    @tip                  = options[:tip]
  end

  def run
    save_service_transaction_items(@appointment_services)
    save_product_transaction_items(@appointment_products)
    save_tip_transaction_item(@appointment_id, @stylist_id, @tip_amount)
  end

  def save_service_transaction_items(appointment_services)
    appointment_services.each { |s| s.save_transaction_item(appointment_id) }
  end

  def save_product_transaction_items(appointment_products)
    appointment_products.each { |p| p.save_transaction_item(appointment_id) }
  end

  def save_tip_transaction_item(appointment_id, stylist_id, tip)
    TransactionItem.create!(
      appointment_id: appointment_id,
      stylist_id: stylist_id,
      label: "Tip",
      price: tip,
      transaction_item_type_id: TransactionItemType.find_or_create_by_code("TIP").id
    )  
  end
end

그 결과 얻는 이점

이제 Appointment 클래스는 다음처럼 한결 간결해집니다:

class Appointment < ActiveRecord::Base 
  has_many :appointment_services, :dependent => :destroy
  has_many :services, :through => :appointment_services
  has_many :appointment_products, :dependent => :destroy
  has_many :products, :through => :appointment_products
  has_many :payments, :dependent => :destroy
  has_many :transaction_items
  belongs_to :client
  belongs_to :stylist
  belongs_to :time_block_type

  def record_transactions
    transaction_items.destroy_all
    if paid_for?
      TransactionRecorder.new(
        appointment_id: id,
        appointment_services: appointment_services,
        appointment_products: appointment_products,
        stylist_id: stylist_id,
        tip: tip
      ).run
    end
  end
end

여전히 테스트할 수 없는 Appointment의 코드를 수정하고는 있지만, 이제 TransactionRecorder의 모든 코드는 테스트할 수 있습니다. 게다가 각 메서드가 인스턴스 변수 대신 인자를 받도록 변경했기 때문에, 개별 메서드를 격리된 상태에서 테스트할 수도 있습니다. 덕분에 우리는 시작 시점보다 훨씬 나은 위치에 서게 된 것입니다.