Right now, the code doesn't work. Your code only validates ISBNs that use Arabic numerals, but some ISBNs can end in an "X". Quoting from the ISBN FAQs:
Why do some ISBNs end in an "X"?
In the case of the check digit, the last digit of the ISBN, the upper case X can appear. The method of determining the check digit for the ISBN is the modulus 11 with the weighting factors 10 to 1. The Roman numeral X is used in lieu of 10 where ten would occur as a check digit.
So you should think about how you’d fix that first.
Comments on the existing
This isn't how I'd write it. Your function doesn’t return a True/False value. The code which acts interactively, and prints things to the user, and the code which validates the ISBN, are both intertwined. I would separate the two. This will make debugging issues like the "X" easier, and you can reuse the validating function later.
For example, if you wanted to enter IBSNs to a database, and verify that the entered numbers were accurate, you could get a True/False value from the function without printing to the console.
Here are some general comments on the existing
It's not clear what the difference between the
number variables are. This is a side-effect of mixing the validation and printing code.
Give it a more specific name, and add a docstring (a way to document functions in Python). Later you may want to validate ISBN-13 codes, and you don't want the namespace cluttered.
There are better ways of handling errors than printing the word "Error" to the console. You could raise a custom
ValueError, or something else.
The line which constructs
num is excessively long: you can use
sum() and a list comprehension to construct it in a more compact way, and it will make it clearer what formula you're using later down the line.
num = num%11 can be replaced by
num %= 11. Just makes things slightly neater.
With those points in mind, here's how I might rewrite your function:
"""A function for validating ISBN-10 codes."""
formatted_num = number.replace("-", "")
if len(formatted_num) != 10:
raise ValueError('The number %s is not a 10-digit string.' % number)
check_sum = sum(int(formatted_num[i]) * (10 - i) for i in xrange(8))
check_sum %= 11
check_digit = 11 - check_sum
if formatted_num[-1] == "X":
return check_digit == 10
return check_digit == int(formatted_num[-1])
If you wanted to be able to tell the user what the correct check digit should be (in the event of an incorrectly formatted string), then you could factor our a
check_digit() function as well.
I leave it as an exercise to write a separate function which interactively prompts the user for an ISBN-10 code, and tells them whether or nots it correctly formatted.
Comments on the rest of the code
Here are some things that concern me in the rest of your code:
You should wrap the interactive code in a
if __name__ == "__main__": block. This means that it will only run if the file is executed directly, but you can also
import the file to just get the function definitions (say, if you wanted to use this ISBN validator in a larger project).
while running == True:. If you're going to compare to booleans, then it's much more idiomatic to write
while running is True:, or even better,
while running:. But that's not a good way to do it.
Instead, suppose you have a
interactive_user_isbn() function which runs the interactive session with the user. Then at the end of that function, if they want to go again, then you can ask them, and if they do, call the function again.
Make the repeat prompt more explicit about what will be repeated, and give them a hint about what they can type to repeat the function.
If they type nothing, then I would err on assuming that they don't want to go again, but that's just my opinion.
# ask the user for an ISBN
# check if it's correct
# pay the user a compliment etc
repeat = input("Do you want to check another number? (y/n)")
if repeat.lower() in ["yes", "y", "ok", "sure"]: