Skip to main content
se eliminó 1 carácter en el cuerpo
Origen Enlace
abulafia
  • 58.7k
  • 3
  • 54
  • 89

De lo contrario el booleano breyrey te va a salir siempre False.

De lo contrario el booleano brey te va a salir siempre False.

De lo contrario el booleano rey te va a salir siempre False.

se añadieron 4361 caracteres en el cuerpo
Origen Enlace
abulafia
  • 58.7k
  • 3
  • 54
  • 89

Mejorar el código (refactorizar)

Mejorar el código (refactorizar)

Corregir bugs

Corregir bugs

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 retornar False inmediatamente cuando se detecta una condición falsa. Por ejemplo, si empezamos viendo que la longitud es mayor de 16 ya podemos retornar False, 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 lista fichas) 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

Mejorar el código (refactorizar)

Corregir bugs

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

Mejorar el código (refactorizar)

Corregir bugs

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 retornar False inmediatamente cuando se detecta una condición falsa. Por ejemplo, si empezamos viendo que la longitud es mayor de 16 ya podemos retornar False, 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 lista fichas) 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
se añadieron 4361 caracteres en el cuerpo
Origen Enlace
abulafia
  • 58.7k
  • 3
  • 54
  • 89

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

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
Origen Enlace
abulafia
  • 58.7k
  • 3
  • 54
  • 89
Loading