Mejorar el código (refactorizar)
La refactorización consiste en reescribir el código de forma que siga haciendo exactamente lo mismo, pero usando otra sintaxis, con el objetivo de hacerlo más simple, más corto, o más fácil de leer y mantener.
Por un lado, volviendo a la forma en que asignas el booleano rey
, le estás dando True
si cierta expresión sale True
, o False
si cierta expresión sale False
. Así pues ¿no es más corto directamente asignarle el resultado de esa expresión?. Es decir, cambiar:
if "brey"in lista_de_fichas and "nrey" in lista_de_fichas:
rey=True
else:
rey=False
simplemente por:
rey = "brey"in lista_de_fichas and "nrey" in lista_de_fichas
Por otro lado, la función valida_tablero()
imprime si el tablero es válido o no, pero creo que sería mejor diseño que no imprima nada, sino que simplemente retorne True
si el tablero es válido o False
si no lo es. De ese modo desde el programa principal recoges ese resultado y es desde el programa principal desde donde imprimes el mensaje. Esto tiene la ventaja de que puedes cambiar el mensaje (por ejemplo traducirlo) sin tener que tocar la función, o tomar cualquier otra acción.
Así pues propongo cambiar:
if longitud<=16 and letra==True and rey==True and numPeonB<=8 and numPeonN<=8:
print("tablero correcto")
else:
print("tablero incorecto")
por:
return longitud<=16 and letra==True and rey==True and numPeonB<=8 and numPeonN<=8
Es más, las comparaciones letra == True
o rey == True
son redundantes. Simplemente puedes usar el valor de letra
o rey
como parte de la expresión, pues son ya en sí un booleano:
return longitud<=16 and letra and rey and numPeonB<=8 and numPeonN<=8
Corregir bugs
Con los cambios anteriores el código sigue teniendo la misma funcionalidad y sólo hemos cambiado un poco algún detalle de cómo está escrito. Pero creo que la funcionalidad también tenía algún bug. Por ejemplo:
Si el tablero tiene dos reyes negros ¿no debería contar como inválido? Sin embargo tu código lo daría por bueno, ya que la cadena "nrey" aparecería entre las fichas (dos veces de hecho, pero eso no se está mirando)
Para resolver este problema habría que contar los reyes igual que se cuentan los peones, y verificar que hay 1 y solo 1.
Si una pieza está colocada en la posición "5r", que sería inválida, aún así el código la daría por buena, pues estás mirando si valor > "8h"
para darla por mala. Pero resulta que "5r"
no es mayor de "8h"
, porque a la hora de comparar cadenas se compara primero el primer carácter, y ya que "5"<"8"
, la cadena completa se considera menor, sin llegar a mirar el segundo carácter.
Para resolver este problema habría que partir en dos la coordenada, y verificar que tanto el número es menor (o igual) de 8 como que la letra es menor (o igual) de h.
Así pues propongo la siguiente implementación que corrige estos problemas. De paso he refactorizado otro poco las condiciones para simplificar la fórmula final del return
, y renombrado algunas de las variables para que el código sea más autoexplicativo.
def valida_tablero(tablero):
# Separar coordenadas y fichas
coordenadas = list(tablero.keys())
fichas = list(tablero.values())
# Comprobar longitud del tablero
longitud_ok = len(tablero) <= 16
# Comprobar coordenadas válidas
coordenadas_ok = True # Suponemos que están bien, mientras no encontremos una mal
for numero, letra in coordenadas: # Al extraer a dos variables se separan los dos caracteres
if numero > "8" or letra > "h":
coordenadas_ok = False
# Contar cuántos reyes y peones hay de cada color
reyes_negros = reyes_blancos = 0
peones_negros = peones_blancos = 0
for ficha in fichas:
if ficha == "npeon":
peones_negros += 1
if ficha == "bpeon":
peones_blancos += 1
if ficha == "nrey":
reyes_negros += 1
if ficha == "brey":
reyes_blancos += 1
# Verificar que el número de piezas es correcto
reyes_ok = reyes_negros == 1 and reyes_blancos == 1
peones_ok = peones_negros <=8 and peones_blancos <=8
# Retornar resultado
return longitud_ok and coordenadas_ok and reyes_ok and peones_ok