Skip to content

[ISBN Verifier] Error in tests that check if the X character is only the verification digit #4014

@Keftcha

Description

@Keftcha

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions