Help refactoring this nasty Ruby if/else statement
So I have this big, hairy if/else statement. I pass a tracking number to it, and then it determines what type of tracking number it is.
How can I simplify this thing? Specifically wanting to reduce the number of lines of codes.
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
Yes, I know. It's nasty.
Try this. I rewrote it using case and regular expressions. I also used :symbols instead of "strings" for the return values, but you can change that back.
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
Depending on whether or not the tracking code is a ruby object, you could also put helper's in it's class definition:
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
Then your conditional becomes:
if number.ups?
# ...
elsif number.dhl?
# ...
elseif number.fedex?
end
One simplified conditional where you are operating on the implied feature of the tracking code. Likewise, if you were to take a looping approach, your loop would also be cleaner:
%w(ups? dhl? fedex?).each do |is_code|
return if number.send(is_code)
end
or even:
%w(ups? dhl? fedex?).each do |is_code|
yield if number.send(is_code)
end
This method looks like it was written for speed. You can use a minhash as a substitute, but I think the code is fairly clean and doesn't require a refactor. Rubyists tend to be disgusted by needless structure, but oftentimes it's needed to model real-world situations and/or provides a performance boost. The keyword should be needless.
链接地址: http://www.djcxy.com/p/58480.html上一篇: 重写isEqual的最佳实践：和散列