帮助重构这个令人讨厌的Ruby if / else语句
所以我有一个很大的毛茸茸的if / else语句。 我将跟踪号码传递给它，然后确定它是什么类型的跟踪号码。
我怎样才能简化这件事？ 特别想减少代码行数。
if num_length < 8 tracking_service = false else if number[1, 1] == 'Z' tracking_service = 'ups' elsif number[0, 1] == 'Q' tracking_service = 'dhl' elsif number[0, 2] == '96' && num_length == 22 tracking_service = 'fedex' elsif number[0, 1] == 'H' && num_length == 11 tracking_service = 'ups' elsif number[0, 1] == 'K' && num_length == 11 tracking_service = 'ups' elsif num_length == 18 || num_length == 20 check_response(number) else case num_length when 17 tracking_service = 'dhlgm' when 13,20,22,30 tracking_service = 'usps' when 12,15,19 tracking_service = 'fedex' when 10,11 tracking_service = 'dhl' else tracking_service = false end end end 是的我知道。 这很讨厌。
试试这个。 我用case和正则表达式重写了它。 我还使用了:symbols而不是"strings"作为返回值，但您可以将其更改回来。
tracking_service = case number when /^.Z/ then :ups when /^Q/ then :dhl when /^96.{20}$/ then :fedex when /^[HK].{10}$/ then :ups else check_response(number) if num_length == 18 || num_length == 20 case num_length when 17 then :dhlgm when 13, 20, 22, 30 then :usps when 12, 15, 19 then :fedex when 10, 11 then :dhl else false end end
根据跟踪代码是否是ruby对象，您还可以在其类定义中添加帮助程序：
class TrackingCode < String # not sure if this makes sense for your use case def ups? self[1,1] == 'Z' end def dhl? self[0,1] == 'Q' end def fedex? self.length == 22 && self[0, 2] == '96' end # etc... end
然后您的条件成为：
if number.ups? # ... elsif number.dhl? # ... elseif number.fedex? end
一个简化的条件，您使用跟踪代码的隐含function。 同样，如果你采用循环方法，你的循环也会更清晰：
%w(ups? dhl? fedex?).each do |is_code| return if number.send(is_code) end
甚至：
%w(ups? dhl? fedex?).each do |is_code| yield if number.send(is_code) end
这个方法看起来像是为了速度而编写的。 您可以使用minhash作为替代，但我认为代码相当干净，不需要重构。 Rubyist倾向于对不必要的结构感到厌恶，但通常需要对现实世界的情况进行建模和/或提供性能提升。 关键字应该是不必要的。
我相信这是非常复杂的，值得拥有自己的方法。
顺便说一句，如果长度是20，则原始函数返回check_response(n)返回的任何内容，然后尝试（并且将始终失败）返回'usps' 。
@lenMap = Hash.new false @lenMap[17] = 'dhlgm' @lenMap[13] = @lenMap[20] = @lenMap[22] = @lenMap[30] = 'usps' @lenMap[12] = @lenMap[15] = @lenMap[19] = 'fedex' @lenMap[10] = @lenMap[11] = 'dhl' def ts n len = n.length return false if len < 8 case n when /^.Z/ return 'ups' when /^Q/ return 'dhl' when /^96....................$/ return 'fedex' when /^[HK]..........$/ return 'ups' end return check_response n if len == 18 or len == 20 return @lenMap[len] end # test code... def check_response n return 'check 18/20 ' end %w{ 1Zwhatever Qetcetcetc 9634567890123456789012 H2345678901 K2345678901 hownowhownowhownow hownowhownowhownow90 12345678901234567 1234567890123 12345678901234567890 1234567890123456789012 123456789012345678901234567890 123456789012 123456789012345 1234567890123456789 1234567890 12345678901 }.each do |s| puts "%32s %s" % [s, (ts s).to_s] end
虽然比jtbandes解决方案更长，但您可能会喜欢这个，因为它更具说明性：
class Condition attr_reader :service_name, :predicate def initialize(service_name, &block) @service_name = service_name @predicate = block end end CONDITIONS = [ Condition.new('ups') { |n| n[1] == 'Z' }, Condition.new('dhl') { |n| n[0] == 'Q' }, Condition.new('fedex') { |n| n[0..1] == '96' && n.size == 22 }, Condition.new('ups') { |n| n[0] == 'H' && n.size == 11 }, Condition.new('ups') { |n| n[0] == 'K' && n.size == 11 }, Condition.new('dhlgm') { |n| n.size == 17 }, Condition.new('usps') { |n| [13, 20, 22, 30].include?(n.size) }, Condition.new('fedex') { |n| [12, 15, 19].include?(n.size) }, Condition.new('dhl') { |n| [10, 11].include?(n.size) }, ] def tracking_service(tracking_number) result = CONDITIONS.find do |condition| condition.predicate.call(tracking_number) end result.service_name if result end
我没有在这里处理check_response方法调用，因为我觉得你应该在其他地方处理它（假设它做了除返回跟踪服务名称之外的其他事情）。