Me dan algunos números de ISBN, por ejemplo, 3-528-03851 (no válido), 3-528-16419-0 (válido). Se supone que debo escribir un progtwig que pruebe si el número ISBN es válido.
Aquí ‘mi código:
def check(isbn): check_digit = int(isbn[-1]) match = re.search(r'(\d)-(\d{3})-(\d{5})', isbn[:-1]) if match: digits = match.group(1) + match.group(2) + match.group(3) result = 0 for i, digit in enumerate(digits): result += (i + 1) * int(digit) return True if (result % 11) == check_digit else False return False
He usado una expresión regular para verificar a) si el formato es válido yb) extrae los dígitos en la cadena ISBN. Si bien parece que funciona, al ser un principiante de Python, estoy ansioso por saber cómo podría mejorar mi código. Sugerencias?
Primero, trata de evitar un código como este:
if Action(): lots of code return True return False
Dale la vuelta para que la mayor parte del código no esté nested. Esto nos da:
def check(isbn): check_digit = int(isbn[-1]) match = re.search(r'(\d)-(\d{3})-(\d{5})', isbn[:-1]) if not match: return False digits = match.group(1) + match.group(2) + match.group(3) result = 0 for i, digit in enumerate(digits): result += (i + 1) * int(digit) return True if (result % 11) == check_digit else False
Hay algunos errores en el código:
Entonces, simplifiquemos esto. Primero, elimine todos los espacios y guiones, y asegúrese de que la expresión regular coincida con toda la línea colocándola en ‘^ … $’. Eso asegura que rechace las cadenas que son demasiado largas.
def check(isbn): isbn = isbn.replace("-", "").replace(" ", ""); check_digit = int(isbn[-1]) match = re.search(r'^(\d{9})$', isbn[:-1]) if not match: return False digits = match.group(1) result = 0 for i, digit in enumerate(digits): result += (i + 1) * int(digit) return True if (result % 11) == check_digit else False
A continuación, vamos a solucionar el problema de dígitos de verificación “X”. Igualar también el dígito de verificación en la expresión regular, de modo que la expresión regular valida toda la cadena, luego convierta el dígito de verificación correctamente.
def check(isbn): isbn = isbn.replace("-", "").replace(" ", "").upper(); match = re.search(r'^(\d{9})(\d|X)$', isbn) if not match: return False digits = match.group(1) check_digit = 10 if match.group(2) == 'X' else int(match.group(2)) result = 0 for i, digit in enumerate(digits): result += (i + 1) * int(digit) return True if (result % 11) == check_digit else False
Finalmente, usar una expresión generadora y max es una forma más idiomática de hacer el cálculo final en Python, y el condicional final se puede simplificar.
def check(isbn): isbn = isbn.replace("-", "").replace(" ", "").upper(); match = re.search(r'^(\d{9})(\d|X)$', isbn) if not match: return False digits = match.group(1) check_digit = 10 if match.group(2) == 'X' else int(match.group(2)) result = sum((i + 1) * int(digit) for i, digit in enumerate(digits)) return (result % 11) == check_digit
Mejora inútil: reemplazar return True if (result % 11) == check_digit else False con return (result % 11) == check_digit
comprueba esto después de que hayas terminado ok 🙂
http://www.staff.ncl.ac.uk/djwilkinson/software/isbn.py
y
http://chrisrbennett.com/2006/11/isbn-check-methods.html
EDITAR: Perdón por lo confuso que no vi la etiqueta de la tarea, pero tal vez después de terminar la tarea pueda ver lo que otros han hecho antes, creo que se puede aprender mucho del código de otros; Lo siento de nuevo 🙁
check_digit puede generar un ValueError si el último carácter no es un dígito decimal. ¿Por qué no sacar el dígito de control con su expresión regular en lugar de usar rebanar? $ , aunque en su caso eso no importará ya que su expresión regular es de ancho fijo). ''.join(match.groups()) , y sacar el check_digit después. Puede hacer la conversión a int s antes de retirarlo, ya que desea convertirlos todos a int s de todos modos. sum() para sumr los elementos. True if (expression) else False generalmente puede reemplazarse con una simple expression . Del mismo modo, False if (expression) else True siempre se puede reemplazar con simplemente not expression Poniendo todo junto:
def check(isbn): match = re.match(r'(\d)-(\d{3})-(\d{5})-(\d)$', isbn) if match: digits = [int(x) for x in ''.join(match.groups())] check_digit = digits.pop() return check_digit == sum([(i + 1) * digit for i, digit in enumerate(digits)]) % 11 return False
La última línea es posiblemente innecesaria, ya que el comportamiento predeterminado sería devolver Ninguno (lo cual es falso), pero los retornos explícitos de algunos caminos y no de otros me parecen un error, así que creo que es más legible dejarlo.
Todo ese material de expresiones regulares es excelente si pertenece a la inspección de cumplimiento de isbn.org.
Sin embargo, si desea saber si lo que los clientes potenciales escriben en su navegador vale la pena incluir en una consulta de su base de datos de libros a la venta, no querrá toda esa bonita capa de uniforme rojo. Simplemente deseche todo menos 0-9 y X … oh, sí, nadie usa la tecla de mayúsculas, así que es mejor que permitamos x también. Entonces, si tiene una longitud de 10 y pasa la prueba de verificación de dígitos, vale la pena hacer la consulta.
De http://www.isbn.org/standards/home/isbn/international/html/usm4.htm
El dígito de control es el último dígito de un ISBN. Se calcula en un módulo 11 con ponderaciones 10-2, utilizando X en lugar de 10, donde diez aparecerían como un dígito de control.
Esto significa que cada uno de los primeros nueve dígitos del ISBN, excluyendo el propio dígito de control, se multiplica por un número que va de 10 a 2 y que la sum resultante de los productos, más el dígito de control, debe ser divisible por 11 sin un rest.
que es una manera muy larga de decir “cada uno de los dígitos se multiplica por un número que va de 10 a 1 y que la sum resultante de los productos debe ser divisible por 11 sin un rest”
def isbn10_ok(s): data = [c for c in s if c in '0123456789Xx'] if len(data) != 10: return False if data[-1] in 'Xx': data[-1] = 10 try: return not sum((10 - i) * int(x) for i, x in enumerate(data)) % 11 except ValueError: # rare case: 'X' or 'x' in first 9 "digits" return False tests = """\ 3-528-03851 3-528-16419-0 ISBN 0-8436-1072-7 0864425244 1864425244 0864X25244 1 904310 16 8 0-473-07480-x 0-473-07480-X 0-473-07480-9 0-473-07480-0 123456789 12345678901 1234567890 0000000000 """.splitlines() for test in tests: test = test.strip() print repr(test), isbn10_ok(test)
Salida:
'3-528-03851' False '3-528-16419-0' True 'ISBN 0-8436-1072-7' True '0864425244' True '1864425244' False '0864X25244' False '1 904310 16 8' True '0-473-07480-x' True '0-473-07480-X' True '0-473-07480-9' False '0-473-07480-0' False '123456789' False '12345678901' False '1234567890' False '0000000000' True '' False
Aparte: un gran sitio de venta de libros conocido aceptará 047307480x, 047307480X y 0-473-07480-X pero no 0-473-07480-x :-O
No olvide (aunque esto puede estar fuera del scope de su asignación) para calcular el dígito de control del ISBN (el dígito final), para determinar si el ISBN es válido y no solo aparentemente válido .
Hay algo de información sobre la implementación del dígito de control en el sitio web ISBN.org , y la implementación debe ser bastante sencilla. Wikipedia ofrece uno de estos ejemplos (suponiendo que ya ha convertido cualquier ASCII “X” a un decimal 10):
bool is_isbn_valid(char digits[10]) { int i, a = 0, b = 0; for (i = 0; i < 10; i++) { a += digits[i]; // Assumed already converted from ASCII to 0..10 b += a; } return b % 11 == 0; }
La aplicación de esto para su tarea se deja, bueno, como un ejercicio para usted.
Tu código es bueno, ¡bien hecho para escribir Python idiomático! Aquí hay algunas cosas menores:
Cuando ves el idioma
result = for elt in : result += elt
Puedes reemplazarlo por una lista de comprensión. En este caso:
result = sum((i+1)*int(digit) for i, digit in enumerate(digits)
o incluso más concisamente:
return sum((i+1)*int(digit) for i, digit in enumerate(digits) % 11 == check_digit
Por supuesto, es un juicio de valor si esto es mejor que el original. Personalmente consideraría que el segundo de estos es el mejor.
Además, los paréntesis adicionales en (result % 11) == check_digit son extraños y realmente no creo que los necesites para mayor claridad. Eso te deja en general con:
def validate(isbn): check_digit = int(isbn[-1]) match = re.search(r'(\d)-(\d{3})-(\d{5})', isbn[:-1]) if match: digits = match.group(1) + match.group(2) + match.group(3) parity = sum((i+1)*int(digit) for i, digit in enumerate(digits) return parity % 11 == check_digit else: return False
Tenga en cuenta que todavía necesita el return False para detectar el caso de que el ISBN no tenga el formato correcto.
El dígito de verificación puede tomar los valores 0-10, según el hecho de que es módulo-11. Hay un problema con la línea:
check_digit = int(isbn[-1])
ya que esto funciona solo para los dígitos 0-9. Necesitará algo para el caso cuando el dígito es ‘X’, y también para la condición de error cuando no es ninguna de las anteriores, de lo contrario, su progtwig se bloqueará.