Skip to main content
Code Review

Return to Answer

deleted 117 characters in body; added 37 characters in body; deleted 4 characters in body; deleted 24 characters in body
Source Link
200_success
  • 145.5k
  • 22
  • 190
  • 479

The doMaths function has no clear purpose — hence its weird name. It acts as both to calculate the expected last digit of a 7-digit input and to validate the last digit of an 8-digit input. Furthermore, it prints its results instead of returning them, hindering reuse of the function.

What you really want to do is share the code that calculates total. You could write a function like this, using slices:

def gtin8_checksum_digit(number):
 """
 Given a 7- or 8-digit string, calculate the expected GTIN-8 checksum.
 >>> gtin8_checksum_digit('3141592')
 '7'
 >>> gtin8_checksum_digit('31415927')
 '7'
 """
 total = sum(3 * int(d) for d in number[0:7:2]) + \
 sum(int(d) for d in number[1:6:2])
 return str(10 - (total % 10))

No floating-point arithmetic should be necessary.

Note the docstring, and in particular, the doctests that make it clear how the function behaves.

To use that function...

if __name__ == '__main__':
 while True:
 try:
 number = input("Enter a 7 or 8 digit number for testing: ")
 int(number) # Trigger possible ValueError
 if 7 <= len(number) ==<= 78: break
 print("Length of number must print(gtin8_checksum_digit(number)be 7 or 8")
 except ValueError:
 break
 print("Only enter numbers.")
 elifif len(number) == 8:
 print("Valid" if gtin8_checksum_digit(number) == number[7] else "Invalid")
 break
 else:
 print("Length of gtin8_checksum_digit(number must be 7 or 8")
 except ValueError:
 print("Only enter numbers.")

The doMaths function has no clear purpose — hence its weird name. It acts as both to calculate the expected last digit of a 7-digit input and to validate the last digit of an 8-digit input. Furthermore, it prints its results instead of returning them, hindering reuse of the function.

What you really want to do is share the code that calculates total. You could write a function like this, using slices:

def gtin8_checksum_digit(number):
 """
 Given a 7- or 8-digit string, calculate the expected GTIN-8 checksum.
 >>> gtin8_checksum_digit('3141592')
 '7'
 >>> gtin8_checksum_digit('31415927')
 '7'
 """
 total = sum(3 * int(d) for d in number[0:7:2]) + \
 sum(int(d) for d in number[1:6:2])
 return str(10 - (total % 10))

No floating-point arithmetic should be necessary.

Note the docstring, and in particular, the doctests that make it clear how the function behaves.

To use that function...

if __name__ == '__main__':
 while True:
 try:
 number = input("Enter a 7 or 8 digit number for testing: ")
 int(number) # Trigger possible ValueError
 if len(number) == 7:
 print(gtin8_checksum_digit(number))
 break
 elif len(number) == 8:
 print("Valid" if gtin8_checksum_digit(number) == number[7] else "Invalid")
 break
 else:
 print("Length of number must be 7 or 8")
 except ValueError:
 print("Only enter numbers.")

The doMaths function has no clear purpose — hence its weird name. It acts as both to calculate the expected last digit of a 7-digit input and to validate the last digit of an 8-digit input. Furthermore, it prints its results instead of returning them, hindering reuse of the function.

What you really want to do is share the code that calculates total. You could write a function like this, using slices:

def gtin8_checksum_digit(number):
 """
 Given a 7- or 8-digit string, calculate the expected GTIN-8 checksum.
 >>> gtin8_checksum_digit('3141592')
 '7'
 >>> gtin8_checksum_digit('31415927')
 '7'
 """
 total = sum(3 * int(d) for d in number[0:7:2]) + \
 sum(int(d) for d in number[1:6:2])
 return str(10 - (total % 10))

No floating-point arithmetic should be necessary.

Note the docstring, and in particular, the doctests that make it clear how the function behaves.

To use that function...

if __name__ == '__main__':
 while True:
 try:
 number = input("Enter a 7 or 8 digit number for testing: ")
 int(number) # Trigger possible ValueError
 if 7 <= len(number) <= 8: break
 print("Length of number must be 7 or 8")
 except ValueError:
 print("Only enter numbers.")
 if len(number) == 8:
 print("Valid" if gtin8_checksum_digit(number) == number[7] else "Invalid")
 else:
 print(gtin8_checksum_digit(number))
Address @Graipher's concern about obfuscation
Source Link
200_success
  • 145.5k
  • 22
  • 190
  • 479

The doMaths function has no clear purpose — hence its weird name. It acts as both to calculate the expected last digit of a 7-digit input and to validate the last digit of an 8-digit input. Furthermore, it prints its results instead of returning them, hindering reuse of the function.

What you really want to do is share the code that calculates total. You could write a function like this, using slices:

def gtin8_checksum_digit(number):
 """
 Given a 7- or 8-digit string, calculate the expected GTIN-8 checksum.
 >>> gtin8_checksum_digit('3141592')
 '7'
 >>> gtin8_checksum_digit('31415927')
 '7'
 """
 total = sum(3 * int(d) for d in number[0:7:2]) + \
 sum(int(d) for d in number[1:6:2])
 return str(10 - (total % 10))

No floating-point arithmetic should be necessary.

Note the docstring, and in particular, the doctests that make it clear how the function behaves.

To use that function...

if __name__ == '__main__':
 while True:
 try:
 number = input("Enter a 7 or 8 digit number for testing: ")
 int(number) # Trigger possible ValueError
 if len(number) == 7:
 print(gtin8_checksum_digit(number))
 break
 elif len(number) == 8:
 print("Valid" if gtin8_checksum_digit(number) == number[7] else
 "Invalid")
 break
 else:
 print("Length of number must be 7 or 8")
 except ValueError:
 print("Only enter numbers.")

The doMaths function has no clear purpose — hence its weird name. It acts as both to calculate the expected last digit of a 7-digit input and to validate the last digit of an 8-digit input. Furthermore, it prints its results instead of returning them, hindering reuse of the function.

What you really want to do is share the code that calculates total. You could write a function like this, using slices:

def gtin8_checksum_digit(number):
 """
 Given a 7- or 8-digit string, calculate the expected GTIN-8 checksum.
 >>> gtin8_checksum_digit('3141592')
 '7'
 >>> gtin8_checksum_digit('31415927')
 '7'
 """
 total = sum(3 * int(d) for d in number[0:7:2]) + \
 sum(int(d) for d in number[1:6:2])
 return str(10 - (total % 10))

No floating-point arithmetic should be necessary.

Note the docstring, and in particular, the doctests that make it clear how the function behaves.

To use that function...

if __name__ == '__main__':
 while True:
 try:
 number = input("Enter a 7 or 8 digit number for testing: ")
 int(number)
 if len(number) == 7:
 print(gtin8_checksum_digit(number))
 break
 elif len(number) == 8:
 print("Valid" if gtin8_checksum_digit(number) == number[7] else
 "Invalid")
 break
 else:
 print("Length of number must be 7 or 8")
 except ValueError:
 print("Only enter numbers.")

The doMaths function has no clear purpose — hence its weird name. It acts as both to calculate the expected last digit of a 7-digit input and to validate the last digit of an 8-digit input. Furthermore, it prints its results instead of returning them, hindering reuse of the function.

What you really want to do is share the code that calculates total. You could write a function like this, using slices:

def gtin8_checksum_digit(number):
 """
 Given a 7- or 8-digit string, calculate the expected GTIN-8 checksum.
 >>> gtin8_checksum_digit('3141592')
 '7'
 >>> gtin8_checksum_digit('31415927')
 '7'
 """
 total = sum(3 * int(d) for d in number[0:7:2]) + \
 sum(int(d) for d in number[1:6:2])
 return str(10 - (total % 10))

No floating-point arithmetic should be necessary.

Note the docstring, and in particular, the doctests that make it clear how the function behaves.

To use that function...

if __name__ == '__main__':
 while True:
 try:
 number = input("Enter a 7 or 8 digit number for testing: ")
 int(number) # Trigger possible ValueError
 if len(number) == 7:
 print(gtin8_checksum_digit(number))
 break
 elif len(number) == 8:
 print("Valid" if gtin8_checksum_digit(number) == number[7] else
 "Invalid")
 break
 else:
 print("Length of number must be 7 or 8")
 except ValueError:
 print("Only enter numbers.")
Source Link
200_success
  • 145.5k
  • 22
  • 190
  • 479

The doMaths function has no clear purpose — hence its weird name. It acts as both to calculate the expected last digit of a 7-digit input and to validate the last digit of an 8-digit input. Furthermore, it prints its results instead of returning them, hindering reuse of the function.

What you really want to do is share the code that calculates total. You could write a function like this, using slices:

def gtin8_checksum_digit(number):
 """
 Given a 7- or 8-digit string, calculate the expected GTIN-8 checksum.
 >>> gtin8_checksum_digit('3141592')
 '7'
 >>> gtin8_checksum_digit('31415927')
 '7'
 """
 total = sum(3 * int(d) for d in number[0:7:2]) + \
 sum(int(d) for d in number[1:6:2])
 return str(10 - (total % 10))

No floating-point arithmetic should be necessary.

Note the docstring, and in particular, the doctests that make it clear how the function behaves.

To use that function...

if __name__ == '__main__':
 while True:
 try:
 number = input("Enter a 7 or 8 digit number for testing: ")
 int(number)
 if len(number) == 7:
 print(gtin8_checksum_digit(number))
 break
 elif len(number) == 8:
 print("Valid" if gtin8_checksum_digit(number) == number[7] else
 "Invalid")
 break
 else:
 print("Length of number must be 7 or 8")
 except ValueError:
 print("Only enter numbers.")
lang-py

AltStyle によって変換されたページ (->オリジナル) /