Challenge แข็งแกร่งของ legacy applications คือ code ไม่ได้เขียนเพื่อ testable ดังนั้น เขียน meaningful tests ยากหรือ impossible
Chicken-and-egg problem: เขียน tests สำหรับ legacy app ต้องเปลี่ยน code แต่เปลี่ยน code ไม่ confident โดยไม่มี tests ก่อน!
จัดการ paradox นี้ยังไง?
หัวข้อนี้หนึ่งในหลายเรื่องใน Working Effectively with Legacy Code ของ Michael Feathers วันนี้ zoom in เทคนิคหนึ่งจากหนังสือชื่อ Sprout Class
Get Ready for Some LEGACY CODE!
ดู ActiveRecord class เก่า Appointment ยาวมาก ในชีวิตจริงยาว 100+ lines
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 Adding Some Features
ถ้าขอเพิ่ม functionality ใหม่ใน transaction-reporting แต่ Appointment class มี dependencies เยอะ testable ได้ด้วย refactoring เยอะ ทำยังไง?
One Option: Just Make the Change
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 That Kind of Sucks
2 ปัญหา:
Appointmentมี responsibilities หลายอย่าง (violate Single Responsibility Principle) หนึ่งคือ recording transactions เพิ่ม transaction-related code ให้ Appointment = ทำให้ code แย่ลงนิดหน่อยอาจเขียน integration test ใหม่ check email ส่งไป แต่ไม่ได้ Appointment class ใน testable state ไม่เพิ่ม unit tests ได้ เพิ่ม untested code ซึ่งแย่ (Michael Feathers define legacy code = "code without tests" เพิ่ม _more_ legacy code ให้ legacy code)
Chunking It Up Is Better
Extract transaction-recording behavior ไป class ของตัวเอง เรียก 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 The Payoff
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 ยัง modify code ใน Appointment ที่ testable ไม่ได้ แต่ตอนนี้ test ทุกอย่างใน TransactionRecorder ได้ และเปลี่ยน functions รับ arguments แทน instance variables test แต่ละ function แยกได้ อยู่ที่ดีกว่าเดิมมาก
