很多非常相似的函數,通心粉代碼修正?

[英]Many very similar functions, spaghetti code fix?


I have approx 11 functions that look like this:

我有大約11個像這樣的函數:

def pending_acceptance(order_fulfillments)
  order_fulfillments.each do |order_fulfillment|
    next unless order_fulfillment.fulfillment_time_calculator.
        pending_acceptance?; collect_fulfillments(
          order_fulfillment.status,
          order_fulfillment
        )
  end
end

def pending_start(order_fulfillments)
  order_fulfillments.each do |order_fulfillment|
    next unless order_fulfillment.fulfillment_time_calculator.
        pending_start?; collect_fulfillments(
          order_fulfillment.status,
          order_fulfillment
        )
  end
end

The iteration is always the same, but next unless conditions are different. In case you wonder: it's next unless and ; in it because RuboCop was complaining about it. Is there a solution to implement it better? I hate this spaghetti code. Something like passing the condition into "iterate_it" function or so...

迭代總是相同的,但是除非條件不同,否則下一個迭代。如果你想知道:這是下一個除非和;因為RuboCop是在抱怨它。有更好的解決方案嗎?我討厭這種意大利面條式的代碼。類似於將條件傳遞給“iterate_it”函數之類的東西……

edit: Cannot just pass another parameter because the conditions are double sometimes:

編輯:不能僅僅傳遞另一個參數,因為條件有時是雙重的:

def picked_up(order_fulfillments)
      order_fulfillments.each do |order_fulfillment|
        next unless
          order_fulfillment.handed_over_late? && order_fulfillment.
              fulfillment_time_calculator.pending_handover?
                collect_fulfillments(
                  order_fulfillment.status,
                  order_fulfillment
                )
      end
    end

edit2: One question yet: how could I slice a symbol, to get a user role from a status? Something like: :deliverer_started => :deliverer or 'deliverer'?

有一個問題:我如何從一個狀態中分割一個符號來獲得一個用戶角色?比如:deliverer_started =>:deliverer還是“deliverer”?

4 个解决方案

#1


1  

you can make array of strings

你可以創建字符串數組。

arr = ['acceptance','start', ...]

in next step:

下一個步驟:

arr.each do |method|

  define_method ( 'pending_#{method}'.to_sym ) do |order_fulfillments|
    order_fulfillments.each do |order_fulfillment|
      next unless order_fulfillment.fulfillment_time_calculator.
      send('pending_#{method}?'); collect_fulfillments(
            order_fulfillment.status,
            order_fulfillment
          )
    end
  end
end

for more information about define_method

有關define_method的更多信息。

#2


4  

You can pass another parameter when you use that parameter to decide what condition to check. Just store all possible conditions as lambdas in a hash:

當您使用該參數來決定檢查哪個條件時,您可以傳遞另一個參數。只需將所有可能的條件存儲為lambda在散列中:

FULFILLMENT_ACTIONS = {
  pending_acceptance: lambda { |fulfillment| fulfillment.fulfillment_time_calculator.pending_acceptance? }, 
  pending_start:      lambda { |fulfillment| fulfillment.fulfillment_time_calculator.pending_acceptance? },
  picked_up:          lambda { |fulfillment| fulfillment.handed_over_late? && fulfillment.fulfillment_time_calculator.pending_handover? }
}

def process_fulfillments(type, order_fulfillments)
  condition = FULFILLMENT_ACTIONS.fetch(type)

  order_fulfillments.each do |order_fulfillment|
    next unless condition.call(order_fulfillment)
    collect_fulfillments(order_fulfillment.status, order_fulfillment)
  end
end

To be called like:

被稱為:

process_fulfillments(:pending_acceptance, order_fulfillments)
process_fulfillments(:pending_start, order_fulfillments)
process_fulfillments(:picked_up, order_fulfillments)

#3


1  

While next is handy it comes late(r) in the code and is thus a bit more difficult to grasp. I would first select on the list, then do the action. (Note that this is only possible if your 'check' does not have side effects like in order_fullfillment.send_email_and_return_false_if_fails).

雖然next很方便,但它在代碼中出現得很晚,因此很難理解。我將首先在列表中選擇,然后執行操作。(請注意,只有當‘check’沒有order_fullfillment.send_email_and_return_謬誤if_failed等副作用時,才可能出現這種情況)。

So if tests can be complex I would start the refactoring by expressing the selection criteria and then pulling out the processing of these items (wich also matches more the method names you have given), somewhere in the middle it might look like this:

因此,如果測試可能很復雜,我將通過表示選擇條件開始重構,然后提取出這些項的處理過程(這也與您給出的方法名稱更匹配),在中間的某個地方可能是這樣的:

def pending_acceptance(order_fulfillments)
  order_fulfillments.select do |o|
    o.fulfillment_time_calculator.pending_acceptance?
  end
end

def picked_up(order_fulfillments)
  order_fulfillments.select do |order_fulfillment|
    order_fulfillment.handed_over_late? && order_fulfillment.
          fulfillment_time_calculator.pending_handover?
  end
end

def calling_code
  # order_fulfillments = OrderFulFillments.get_from_somewhere
  # Now, filter
  collect_fulfillments(pending_start      order_fulfillments)
  collect_fulfillments(picked_up          order_fulfillments)
end

def collect_fullfillments order_fulfillments
  order_fulfillments.each {|of| collect_fullfillment(of) }
end

You'll still have 11 (+1) methods, but imho you express more what you are up to - and your colleagues will grok what happens fast, too. Given your example and question I think you should aim for a simple, expressive solution. If you are more "hardcore", use the more functional lambda approach given in the other solutions. Also, note that these approaches could be combined (by passing an iterator).

你仍然會有11種(+1)方法,但我希望你能更多地表達你的想法——你的同事也會很快地摸索發生的事情。鑒於你的例子和問題,我認為你應該以一個簡單的、有表現力的解決方案為目標。如果您更“硬核”,則使用其他解決方案中給出的更實用的lambda方法。另外,請注意,這些方法可以組合(通過傳遞迭代器)。

#4


0  

You could use something like method_missing.

你可以使用method_missing這樣的方法。

At the bottom of your class, put something like this:

在你的課的最后,放一些這樣的東西:

def order_fulfillment_check(method, order_fulfillment)
  case method
    when "picked_up" then return order_fulfillment.handed_over_late? && order_fulfillment.fulfillment_time_calculator.pending_handover?
    ...
    ... [more case statements] ...
    ...
    else return order_fulfillment.fulfillment_time_calculator.send(method + "?")
  end
end

def method_missing(method_name, args*, &block)
  args[0].each do |order_fulfillment|
    next unless order_fulfillment_check(method_name, order_fulfillment); 
    collect_fulfillments(
        order_fulfillment.status,
        order_fulfillment
      )
  end
end

Depending on your requirements, you could check if the method_name starts with "pending_".

根據您的需求,您可以檢查method_name是否以“pending_”開頭。

Please note, this code is untested, but it should be somewhere along the line.

請注意,這段代碼沒有經過測試,但是它應該在這條線上的某個地方。

Also, as a sidenote, order_fulfillment.fulfillment_time_calculator.some_random_method is actually a violation of the law of demeter. You might want to adress this.

另外,作為sidenote, order_履行。履行ment_time_calculator。some_random_method實際上違反了demeter定律。您可能想對此進行補充。


注意!

本站翻译的文章,版权归属于本站,未经许可禁止转摘,转摘请注明本文地址:https://www.itdaan.com/blog/2016/08/31/72efe50787f71d799027709cd59c398d.html



 
粤ICP备14056181号  © 2014-2021 ITdaan.com