Donc j'ai cette grande déclaration velue si / sinon. Je passe un numéro de suivi à ce sujet, puis il détermine le type de numéro de suivi qu'il est.
Comment puis-je simplifier cette chose? Spécifiquement voulant réduire le nombre de lignes de code. oui, je sais. C'est méchant.
5 Réponses :
Cette méthode semble avoir été écrite à la vitesse. Vous pouvez utiliser une minouhash comme substitut, mais je pense que le code est assez propre et ne nécessite pas de refacteur. Les rubyistes ont tendance à être dégoûtés par une structure inutile, mais elle est nécessaire pour modéliser des situations du monde réel et / ou fournir un coup de pouce de performance. Le mot clé doit être inutile.
Bien que vous puissiez avoir raison, une structure supplémentaire est parfois nécessaire, ce code pourrait être écrit plus proprement. Voir ma réponse.
Cela n'a pas l'air d'avoir été écrits à la vitesse. Je ne vois aucune optimisation claire ou quoi que ce soit. Cela me semble que le moyen le plus évident de vérifier un tas de conditions.
Essayez ceci. Je l'ai réécrit en utilisant case code> et expressions régulières. J'ai également utilisé : symboles code> au lieu de "chaînes" code> pour les valeurs de retour, mais vous pouvez modifier cela. 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
Je remplacerais les lambdas et le singe se répandant avec: /^96(.20r.20r.20rk] ±/ et /^[Hk](. {10b> numéro
Mais il semble mal, dans le numéro num_length == 18 || Num_Length == 20 cas, il se comporte différent du code de l'OP. Ou n'est-ce pas?
@Radiospiel Je n'ai pas pensé à ce morceau de code pendant 3 ans ... mais on dirait que tu as raison. Bien que le code de l'OP ait la longueur == 20 cas énumérés comme USPS , même s'il aurait déjà été géré par check_reesponse ... quand même, je fais confiance à l'OP ils avaient besoin :)
C'est assez cool. J'espère que vous ne serez pas offensé que j'ai emprunté la majeure partie de votre solution ci-dessous, mais la structurée comme une éventail de Procs.
@DavidFoster aucune infraction prise. Pouvez-vous croire que cette réponse a été écrite il y a 9 ans?
Ha! Ces choses sont sorta intemporelles, comme lire un puzzle d'échecs dans le journal.
tandis que pendant plus longtemps que la solution JTBANES, vous pouvez aimer cela car il est un peu plus déclaratif:
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
Selon que le code de suivi soit ou non un objet Ruby, vous pouvez également mettre l'aide de la classement de la classe: puis votre conditionnel devient: Un conditionnel simplifié où vous utilisez la fonction impliquée du code de suivi. De même, si vous deviez adopter une approche en boucle, votre boucle serait également plus propre: ou même:
Je suis d'accord avec cette approche, encapsulant les conditionnels difficiles à suivre pour déterminer l'expéditeur permet un code beaucoup plus lisible. Cela rendra des tests beaucoup plus raisonnables et plus faciles à comprendre (par exemple pour la moqueur). Enfin, si l'un des expéditeurs modifie la logique pour générer des numéros de suivi, les modifications apportées à votre code sont minimes.
Je crois que cela est suffisamment complexe pour mériter sa propre méthode.
BTW, si la longueur est 20, la fonction d'origine renvoie tout ce que check_response (n) code> retourne, pourtant, puis tente (et va Toujours échouer) Retourner 'USPS' CODE>. P> @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
Qu'est-ce que
check_response (numéro) faire? En dehors de cela, le reste pourrait réside dans une méthode get_tracking_service .