[ISBN Verifier] Error in tests that check if the X character is only the verification digit #4014
Description
Hi,
During a review, the person ask me if his code check if the X character is only acceptable at the end of the ISBN. So I look at the test and I see this test:
def test_x_is_only_valid_as_a_check_digit(self):
self.assertIs(is_valid("3-598-2X507-9"), False)I think this is the test that test it. But here is the problem I have.
The sum % 11 is not equal to 0.
Here is his solution:
def is_valid(isbn):
isbn = isbn.replace('-', '')
if len(isbn) != 10:
return False
sum = 0
for index, digit in enumerate(list(isbn)):
if digit.isdecimal():
sum += int(digit) * (10 - index)
elif digit == 'X':
sum += 10
else:
return False
return sum % 11 == 0 Here the operation his code is doing:
3 * 10 + 5 * 9 + 9 * 8 + 8 * 7 + 2 * 6 + 10 + 5 * 4 + 0 * 3 + 7 * 2 + 9 * 1 = 268
268 % 11 = 4
As you can see the 10 (replacing the X value) is not multiplied (this is how he write his solution). Because of this his test pass but not for the good reason. It pass because the sum % 11 != 0 and not because X is valid only as a check character.
I think we should add the test case where the ISBN is "3-598-2X507-5". This is a case where the sum is 264 (and 264 % 11 == 0). Doing so will handle that sum % 11 == 0 and the X character is not at the end.