대형 컨트롤러 메소드를 모델로 리팩터링하십시오.

Question

PHP 코드베이스에서 포팅 된 레일 애플리케이션이 있습니다. 기본적으로 장바구니에있는 항목을 기준으로 총 가격을 계산하는 긴 컨트롤러 방법이 있습니다. 이것은 PHP 코드에서 직접 포팅 된 레거시 메소드입니다. 코드 위

def total 
    order = @cart.get_or_create_order 
    order_contents = order_contents_for(order) 

    discounts = { 
     events: {}, 
     subjects: {}, 
     products: {} 
    } 

    adjusted_pricing = {} 
    free = false 
    shipping = 0 
    total = 0 

    # Logic for computing shipping price 

    # Construct discount hash 

    order_contents.each do |item| 
     if (item.variant_price.present?) 
     price = item.variant_price 
     end 
     else 
     price = item.price 
     end 

     price_adjustments = {} 
     popped_from = [] 

     # it's the issue due to legacy database structure, 
     # product_id, subject_id and event_id is each column in 
     # the database 
     if (discounts[:products][item.product_id]) 
      price_adjustments = discounts[:products][item.product_id] 
      discounts[:products].delete(item.product_id) 
      popped_from = [:products, item.product_id] 
     elsif (discounts[:subjects][item.subject_id]) 
      price_adjustments = discounts[:subjects][item.subject_id] 
      discounts[:subjects].delete(item.subject_id) 
      popped_from = [:subjects, item.subject_id] 
     elsif (discounts[:events][item.event_id]) 
      price_adjustments = discounts[:events][item.event_id] 
      discounts[:events].delete(item.event_id) 
      popped_from = [:events, item.event_id] 
     end 

     if (adjustment = price_adjustments['$']) 
      adjusted_price = price + adjustment 
     elsif (adjustment = price_adjustments['%']) 
      adjusted_price = price + price * (adjustment/100.0) 
      discounts[popped_from[0]][popped_from[1]] = price_adjustments 
     else 
      adjusted_price = price 
     end 

     adjusted_pricing[item.product_id] = {price: adjusted_price, discount: price - adjusted_price} 

     total += adjusted_price 
    end 

    total += shipping 
end 

는 방법에 대한 코드의 거대한 조각이다, 그래서 그것을 리팩토링과 모델 price_calculator로 이동하기 위해 노력하고있어.

def calculate_total_for(order) 
    order_contents = order.current_order_contents 
    product_adjustments = order.product_adjustments 

    shipping = calculate_shipping_price(order_contents, product_adjustments) 

    discounts = construct_discount_hash(product_adjustments) 

    adjusted_pricing = construct_adjusted_price_hash(discounts, order_contents) 

    total_price = adjusted_pricing.inject(0) { |total, (k, v)| total + v[:price] } 

    { 
    total_price: total_price + shipping, 
    shipping: shipping, 
    adjusted_pricing: adjusted_pricing 
    } 
end 

나는 자신의 클래스로 이전 거대한 방법을 이동하는 과정에서 기본적으로 여전히했고, 그 클래스와 같은 calculate_shipping_price, construct_discount_hash에 별도의 개인 방법으로 논리를 분할 무엇.

좋은 코드와 거리가 멀다는 것을 알고 있습니다. 가독성 측면에서 좋은 개인 메소드로 분해하십시오. 그러나 테스트를하기가 점점 어려워지고 있습니다. 바라건대, 여기 누군가가 조언이나 지침을 줄 수 있는데, 루비의 코드 위에서 리펙토링하는 가장 좋은 방법은 무엇일까요?

추 신 : 저는 루비/레일 레알을 처음 접했습니다. 이전에 대부분 C#/Javascript로 작성 했으므로 익숙하지 않은 일을하는 관용구 나 루비가 있습니다.

Answer 1

앞에서 언급 한 예제에서는 Extract Class from Method 리팩토링을 사용하고 모델의 모든 항목을 이동하는 대신 Service Object을 사용합니다. 여기

이 작업을 수행하는 방법에 대한 개요입니다, 물론 나는 당신을 위해 구현을 떠나 :

class Test 
    def total 
    order = @cart.get_or_create_order 
    order_contents = order_contents_for(order) 

    discounts = { 
     events: {}, 
     subjects: {}, 
     products: {} 
    } 

    service = CalculateTotal.new(order, order_contents, discounts) 

    if service.success? 
     # Success logic 
    else 
     flash[:error] = service.error 
     # Failure logic 
    end 
    end 
end 

class CalculateTotal 
    attr_reader :success, :error 

    def initialize(order, order_contents, discounts) 
    @order = order 
    @order_contents = order_contents 
    @discounts = discounts 
    end 

    def call 
    sum_orders + adjusted_prices + shipping 
    end 

    def success? 
    !@error 
    end 

    private 

    def sum_orders 
    # Logic 

    if something_fails 
     @error = 'There was an error calculating the price' 
    end 

    # Logic 
    end 

    def adjusted_prices 
    # Logic 
    end 

    def shipping 
    # Logic 
    end 
end