这个校验位方法可以重构吗？答案

【问题标题】：Can this check digit method be refactored?这个校验位方法可以重构吗？
【发布时间】：2011-08-29 15:33:16
【问题描述】：

我有以下方法对跟踪号进行校验位，但感觉很冗长/草率。是否可以重构并进行大体清理？

我正在运行 Ruby 1.8.7。

def is_fedex(number)
  n = number.reverse[0..14]

  check_digit = n.first.to_i

  even_numbers = n[1..1].to_i + n[3..3].to_i + n[5..5].to_i + n[7..7].to_i + n[9..9].to_i + n[11..11].to_i + n[13..13].to_i

  even_numbers = even_numbers * 3

  odd_numbers = n[2..2].to_i + n[4..4].to_i + n[6..6].to_i + n[8..8].to_i + n[10..10].to_i + n[12..12].to_i + n[14..14].to_i

  total = even_numbers + odd_numbers

  multiple_of_ten = total + 10 - (total % 10)

  remainder = multiple_of_ten - total

  if remainder == check_digit
    true
  else
    false
  end
end

编辑：以下是有效和无效的数字。

有效：9612019950078574025848

无效：9612019950078574025847

【问题讨论】：

布尔表达式已经计算为true 或false，所以你可以直接使用remainder == check_digit，而不是最后的if。
你有有效号码和无效号码的例子吗？这允许回答的人在发布之前测试他们的代码。

标签： ruby refactoring

【解决方案1】：

def is_fedex(number)
  total = (7..20).inject(0) {|sum, i| sum + number[i..i].to_i * ( i.odd? ? 1 : 3 ) }
  number[-1].to_i == (total / 10.0).ceil * 10 - total
end

我认为您应该保留您的代码。虽然它不是惯用的或聪明的，但它是你几个月后最容易理解的一种。

【讨论】：

【解决方案2】：

我不是 ruby 程序员，所以如果有任何语法错误，我深表歉意，但您应该了解总体思路。我看到了一些事情：首先，您不需要对数组进行切片，单个索引就足够了。其次，您可以执行以下操作，而不是拆分偶数和奇数：

total = 0
for i in (1..14)
  total += n[i].to_i * ( i % 2 == 1 ? 1 : 3 )
end

第三，余数可以简化为 10 - (total % 10)。

【讨论】：

你可以用i.odd?代替i % 2 == 1

【解决方案3】：

我知道您正在运行 1.8.7，但这是我尝试使用 each_slice 并结合注入的 1.9.2 功能：

def is_fedex(number)
  total = number.reverse[1..14].split(//).map(&:to_i).each_slice(2).inject(0) do |t, (e,o)|
    t += e*3 + o 
  end
  10 - (total % 10) == number[-1].to_i 
end

它通过了两个测试

【讨论】：

哦，太好了，我不知道你可以同时使用each_with_index 和inject
使用 each_slice(2) 而不是 each_with_index，您可以处理成对的奇数和偶数索引数。无需重复测试均匀性。

【解决方案4】：

试试这个：

#assuming number comes in as a string
def is_fedex(number)
  n = number.reverse[0..14].scan(/./)
  check_digit = n[0].to_i
  total = 0
  n[1..14].each_with_index {|d,i| total += d.to_i * (i.even? ? 3 : 1) }
  check_digit == 10 - (total % 10)
end

> is_fedex("12345678901231")  => true

编辑按照 Kevin 的建议加入简化的余数逻辑

【讨论】：

【解决方案5】：

这样的？

def is_fedex(number)
  even_arr, odd_arr = number.to_s[1..13].split(//).map(&:to_i).partition.with_index { |n, i| i.even? }
  total = even_arr.inject(:+) * 3 + odd_arr.inject(:+)

  number.to_s.reverse[0..0].to_i == (total + 10 - (total % 10)) - total
end

如果你能给我一个有效和无效的号码，我可以测试它是否有效，并可能进一步调整:)

【讨论】：

有效：9612019950078574025848 无效：9612019950078574025847
是的 enumerable 有各种有趣的技巧......虽然我认为你可能已经超越了自己试图将偶数和奇数分开......而你可以循环并做数学一口气。 :)
@DGM 是肯定的.. 我应该先查看整个原始示例，而不是逐行重构它。

【解决方案6】：

这个函数应该：

def is_fedex(number)
  # sanity check
  return false unless number.length == 15

  data = number[0..13].reverse
  check_digit = number[14..14].to_i

  total = (0..data.length-1).inject(0) do |total, i|
    total += data[i..i].to_i * 3**((i+1)%2)
  end

  (10 - total % 10) == check_digit
end

算术表达式3**((i+1)%2) 可能看起来有点复杂，但本质上与(i.odd? ? 1 : 3) 相同。两种变体都是正确的，您使用哪种方式取决于您（并且可能会影响速度...）

另外请注意，如果您使用 Ruby 1.9，您可以使用 data[i] 而不是 Ruby 1.8 所需的 data[i..i]。

【讨论】：