检查ISBN号码是否正确

Question

我有一些ISBN号码，比如 3-528-03851（无效），还有 3-528-16419-0（有效）。我需要写一个程序来检查这些ISBN号码是否有效。

这是我的代码：

def check(isbn):
    check_digit = int(isbn[-1])
    match = re.search(r'(\d)-(\d{3})-(\d{5})', isbn[:-1])

    if match:
        digits = match.group(1) + match.group(2) + match.group(3)
        result = 0

        for i, digit in enumerate(digits):
          result += (i + 1) * int(digit)

        return True if (result % 11) == check_digit else False

    return False

我用了正则表达式来检查：a) 格式是否正确，b) 从ISBN字符串中提取数字。虽然看起来可以用，但作为一个Python初学者，我很想知道如何改进我的代码。有什么建议吗？

Answer 1

• check_digit的初始化可能会引发一个ValueError错误，如果最后一个字符不是数字的话。为什么不直接用正则表达式提取检查数字，而不是用切片呢？
• 你可能应该用match而不是search，除非你想允许前面有一些任意的杂项字符。（另外，作为一个经验法则，我建议在结尾加上$，不过在你的情况下，这个正则表达式是固定长度的，所以这点不太重要。）
• 与其手动列出分组，不如直接用''.join(match.groups())，然后再提取check_digit。你还可以在提取之前就把它们转换成int类型，因为你本来就想把它们都转换成int。
• 你的for循环可以用列表推导式或生成器表达式来替代。只需用sum()来加总这些元素。
• True if (expression) else False通常可以直接用expression来替代。同样，False if (expression) else True也可以直接用not expression来替代。

把这些都放在一起：

def check(isbn):
    match = re.match(r'(\d)-(\d{3})-(\d{5})-(\d)$', isbn)
    if match:
        digits = [int(x) for x in ''.join(match.groups())]
        check_digit = digits.pop()
        return check_digit == sum([(i + 1) * digit
                                  for i, digit in enumerate(digits)]) % 11
    return False

最后一行可以说是多余的，因为默认行为是返回None（这被认为是“假”的），但从某些路径明确返回而从其他路径不返回，看起来像是个bug，所以我觉得保留它会让代码更易读。

Answer 2

没必要的改进：把 return True if (result % 11) == check_digit else False 替换成 return (result % 11) == check_digit

Answer 3

首先，尽量避免写这样的代码：

if Action():
    lots of code
    return True
return False

把代码结构调整一下，尽量不要让大部分代码嵌套在里面。这样我们可以得到：

def check(isbn):
    check_digit = int(isbn[-1])
    match = re.search(r'(\d)-(\d{3})-(\d{5})', isbn[:-1])

    if not match:
        return False

    digits = match.group(1) + match.group(2) + match.group(3)
    result = 0

    for i, digit in enumerate(digits):
      result += (i + 1) * int(digit)

    return True if (result % 11) == check_digit else False

代码里有一些问题：

• 如果校验位不是整数，比如"0-123-12345-Q"，这会引发一个错误，而不是返回假。
• 如果校验位是10（用"X"表示），这也会引发一个错误，而不是返回真。
• 这段代码假设ISBN总是以"1-123-12345-1"的格式分组，但实际上ISBN的分组是任意的。例如，"12-12345-12-1"也是有效的。具体可以参考这个链接：http://www.isbn.org/standards/home/isbn/international/html/usm4.htm。
• 这段代码假设ISBN是用连字符分隔的，但空格也是有效的。
• 它没有检查是否有多余的字符，比如'0-123-4567819'会返回真，忽略了最后多出来的1。

所以，让我们简化一下。首先，去掉所有的空格和连字符，并确保正则表达式匹配整行，通过在前后加上'^...$'来实现。这样可以确保它拒绝那些过长的字符串。

def check(isbn):
    isbn = isbn.replace("-", "").replace(" ", "");
    check_digit = int(isbn[-1])
    match = re.search(r'^(\d{9})$', isbn[:-1])
    if not match:
        return False

    digits = match.group(1)

    result = 0
    for i, digit in enumerate(digits):
      result += (i + 1) * int(digit)

    return True if (result % 11) == check_digit else False

接下来，解决"X"校验位的问题。在正则表达式中也匹配校验位，这样整个字符串都能通过正则验证，然后正确转换校验位。

def check(isbn):
    isbn = isbn.replace("-", "").replace(" ", "").upper();
    match = re.search(r'^(\d{9})(\d|X)$', isbn)
    if not match:
        return False

    digits = match.group(1)
    check_digit = 10 if match.group(2) == 'X' else int(match.group(2))

    result = 0
    for i, digit in enumerate(digits):
      result += (i + 1) * int(digit)

    return True if (result % 11) == check_digit else False

最后，使用生成器表达式和max是Python中更符合习惯的方式来进行最终计算，而且最后的条件判断也可以简化。

def check(isbn):
    isbn = isbn.replace("-", "").replace(" ", "").upper();
    match = re.search(r'^(\d{9})(\d|X)$', isbn)
    if not match:
        return False

    digits = match.group(1)
    check_digit = 10 if match.group(2) == 'X' else int(match.group(2))

    result = sum((i + 1) * int(digit) for i, digit in enumerate(digits))
    return (result % 11) == check_digit