Sprout 클래스를 사용하여 루비 리팩토링하기

일부 레거시 애플리케이션으로 작업할 때 가장 어려운 문제 중 하나는 코드가 테스트 가능하도록 작성되지 않았다는 것입니다. 따라서 의미 있는 테스트를 작성하는 것은 어렵거나 불가능합니다 .

그것은 닭과 달걀의 문제입니다. 레거시 애플리케이션에 대한 테스트를 작성하려면 코드를 변경해야 하지만 먼저 테스트를 작성하지 않고는 자신 있게 코드를 변경할 수 없습니다!

이 역설을 어떻게 처리합니까?

이것은 Michael Feathers의 뛰어난 Working Effectively with Legacy Code에서 다룬 많은 주제 중 하나입니다. . 오늘은 새싹 클래스라는 책에서 특정 기술을 확대해 보겠습니다. .

기존 코드를 준비하세요!

Appointment라는 오래된 ActiveRecord 클래스를 살펴보겠습니다. . 꽤 길고 실제로는 100줄 더 깁니다.

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 클래스에 많은 리팩토링 없이 테스트할 수 있는 종속성이 너무 많습니다. 어떻게 진행하나요?

한 가지 옵션은 변경하는 것입니다.

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 다양한 책임이 있으며(단일 책임 원칙 위반) 이러한 책임 중 하나는*거래 기록입니다. <엠>. Appointment에 더 많은 거래 관련 코드를 추가하여 클래스, **코드를 조금 더 나쁘게 만들고 있습니다. *.
새로운 통합 테스트를 작성하고 이메일이 통과했는지 확인할 수 있지만 Appointment이 없기 때문에 클래스가 테스트 가능한 상태이므로 단위 테스트를 추가할 수 없습니다. 테스트되지 않은 코드를 더 추가할 예정입니다. , 당연히 나쁜 것입니다. (Michael Feathers는 실제로 레거시 코드를 정의합니다. "테스트가 없는 코드"로 표시되므로 레거시 코드에 레거시 코드를_more_ 추가합니다.)

덩어리로 묶는 것이 더 좋습니다

단순히 새 코드를 인라인으로 추가하는 것보다 더 나은 솔루션은 트랜잭션 기록 동작을 자체 클래스로 추출하는 것입니다. 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의 모든 것을 테스트할 수 있습니다. , 인스턴스 변수를 사용하지 않고 인수를 허용하도록 각 함수를 변경했기 때문에 각 함수를 개별적으로 테스트할 수도 있습니다. 그래서 우리는 시작했을 때보다 훨씬 더 나은 위치에 있습니다.