Ggthjy

I’ve recently coded a way of checking a 10-digit ISBN.

I was wondering if the code works to check the numbers, and if there are any flaws in my code.

def isbn(number):

    try:

        numberreal = number

        #keeps real number with "-"

        numberzero = number.replace("-", "")

        #this makes sure python doesnt drop the first 0 if it is at the start

        number = number.replace("-", "")

        number = int(number)

        number = str(numberzero)

        print("The ISBN Number Entered is", numberreal)

        num = int(number[0]) * 10 + int(number[1]) * 9 + int(number[2]) * 8 + int(number[3]) * 7 + int(number[4]) * 6 + int(number[5]) * 5 + int(number[6]) * 4 + int(number[7]) * 3 + int(number[8]) * 2

        num = num%11

        checknum = 11 - num



        print("The Check Digit Should Be", checknum, "and the one in the code provided was", number[9])

        if int(checknum) == int(number[9]):

            print("The Check Digit Provided Is Correct")

        else:

            print("The Check Digit Provided Is Incorrect")

    except ValueError:

        print("Not Valid Number")

        error()

    except IndexError:

        print("Not 10 Digits")

        error()







def error():

    print("Error")



running = True

while running == True:

    isbn(input("What is the isbn 10 digit number? "))

    restart = input("Do You Want Restart?")

    restart = restart.lower()

    if restart in ("yes", "y", "ok", "sure", ""):

        print("Restartingn" + "-" * 34)

    else:

        print("closing Down")

        running = False

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 isbn() function

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 isbn() function:

• It's not clear what the difference between the numberzero, numberreal and 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.




• 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:

def validate_isbn10(number):

    """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

    else:

        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).



• 
  

  The line 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.

  
  
  

  Something like:

  
  
  
  
  

  def interactive_user_isbn():
  
      # 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"]:
  
          print("n")
  
          interactive_user_isbn()
  
      else:
  
          print("Okay, bye!")