Hacer que tu código funcione
Hay dos errores en tu código. Primero, la forma en que verificas si están los reyes, que haces así:
if "brey"in tablero and "nrey"in tablero:
rey=True
else:
rey=False
Estás buscando las palabras "brey" y "nrey" en la variable tablero
, que es un diccionario. Eso lo busca únicamente en las claves y no en los valores, que es donde realmente están. Por tanto deberías buscarlo en lista_de_fichas
. Así:
if "brey"in lista_de_fichas and "nrey" in lista_de_fichas:
rey=True
else:
rey=False
De lo contrario el booleano rey
te va a salir siempre False
.
Por otro lado a la hora de validar el tablero completo tienes una serie de and
, pero al final un or
:
if longitud<=16 and letra==True and rey==True and numPeonB<=8 or numPeonN<=8:
Debido a ese or
y a que la expresión completa se evalúa de izquierda a derecha, aún si todas las condiciones previas son falsas (por ejemplo la condición rey==True
era falsa), no importa con tal de que la última condición (numPeonN<=8
) sea cierta, pues al final se estaría haciendo or
entre False
y True
, por lo que daría True
.
Esta era la razón por la que siempre te daba true (sólo poniendo más de 8 peones negros te daría False
). La solución es cambiar ese último or
también por un and
:
if longitud<=16 and letra==True and rey==True and numPeonB<=8 and numPeonN<=8:
Con estos dos cambios tu código ya funcionaría. Pero aún puede mejorarse, cosa que haré en ediciones siguientes.
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
Segunda refactorización y otras mejoras
Sobre la versión con los bugs corregidos se pueden mejorar algunas cosas, aunque ya estaría cambiando también en parte la funcionalidad, por lo que no es una mera refactorización.
Primero los detalles de refactorización:
En lugar de ir calculando cada booleano para al final retornar un
and
de todos ellos, se puede retornarFalse
inmediatamente cuando se detecta una condición falsa. Por ejemplo, si empezamos viendo que la longitud es mayor de 16 ya podemos retornarFalse
, pues el tablero no es correcto, y no merece la pena perder el tiempo mirando si cumple o no el resto de propiedades.Solo si pasa todas las pruebas, al final del todo, habrá un simple
return True
.Para calcular el número de peones y de reyes de cada color, en lugar de un bucle en que vamos incrementando "a mano" una serie de contadores, podemos hacer uso de una clase que nos da Python, llamada
collections.Counter
que cuenta los elementos diferentes que hay en una lista que le pases (en este caso sería la listafichas
) y te retorna un diccionario con los contadores. Las claves serían los nombres de las fichas, y los valores cuántas ha contado de cada una.
Además, se me ocurre la siguiente mejora. En lugar de hacer que la función retorne meramente un booleano, hacer que retorne una tupla de booleano más cadena. El booleano dice si el tablero es correcto o no, y la cadena contiene información sobre qué falló, si era incorrecto.
Esta sería la nueva implementación:
def valida_tablero(tablero):
# Separar coordenadas y fichas
coordenadas = list(tablero.keys())
fichas = list(tablero.values())
# Comprobar longitud del tablero
if len(tablero) > 16:
return False, "Tablero demasiado largo"
# Comprobar coordenadas válidas
for numero, letra in coordenadas: # Al extraer a dos variables se separan los dos caracteres
if numero > "8" or letra > "h":
return False, "Coordenadas erroneas"
# Contar cuántos reyes y peones hay de cada color
contadores = Counter(fichas)
# Solo un rey de cada color
if contadores["brey"] != 1 or contadores["nrey"]!=1:
return False, "Número de reyes incorrecto"
# Menos de 8 peones de cada color
if contadores["bpeon"]>8 or contadores["npeon"]>8:
return False, "Número de peones incorrecto"
return True, "Tablero correcto"
Fíjate que la función sigue sin imprimir nada, sino que retorna el mensaje para que sea el programa principal quien lo imprima. Ejemplos de uso:
tablero_armado={"1h":"nrey","6c":"breyna","2g":"npeon","5h":"nreyna","3e":"brey"}
ok, msg = valida_tablero(tablero_armado)
print(msg)
-------------------------------
Tablero correcto
tablero_armado={"1h":"nrey","6c":"breyna","2g":"npeon","5h":"nreyna","3e":"npeon"}
ok, msg = valida_tablero(tablero_armado)
print(msg)
-------------------------------
Número de reyes incorrecto