-1

Acabo de hacer uno de mis primeros programas en Python como principiante y quisiera saber si está bien optimizado. Por bien optimizado me refiero a si hay funciones en Python que puedan reemplazar algunas secciones de mi código y así simplificarlo.

# importations
import string
# definitions
names = []
punct_signs = list(string.punctuation) + list(string.digits)

while True:
    # solicito el texto
    text = input("Introduce tu texto: \n")
    # lo analizo y cambio los signos de puntuacion por espacios
    text = "".join(letter if letter not in punct_signs and letter.isalpha()==True else " " for letter in text)
    # elimino todas las palabras repetidas y guardo todas las palabras distintas en una lista ordenado alfabeticamente
    words = sorted(set(text.lower().split()))

    # solicito los nombres en el texto
    print("Introduce los nombres de personas, lugares o marcas presentes en tu texto: \n(escribe 'listo' cuando termines de escribir los nombres)")

    while True:
        # tomo el nombre
        name = input().split()

        def evaluate(name):
            # descarto nombres repetidos
            if name in names:
                return 1
            # establezco la palabra para finalizar 
            elif "listo" in name:
                return 2
            else:
                # analizo el nombre ingresado para filtrarlos de nombres no validos
                for a in name:
                    if a.isdigit():
                        return 3
                    elif a.isalpha()==False:
                        for i in a:
                            if i in punct_signs and i!="'":
                                return 3
                    # en el caso de que el nombre no este en el texto
                    elif a not in words:
                        return 4
        # descarto nombres repetidos
        if evaluate(name)==1:
            print("el nombre ya se ha introducido")
        elif evaluate(name)==2:
            break
        elif evaluate(name)==3:
            print("el nombre introducido no es valido")
        elif evaluate(name)==4:
            print("el nombre no esta en el texto")
            continue
        # si pasa todas las pruebas, lo añade a la lista de nombres
        else:
            names.append(name)
    # aplano la lista
    names= [item for lista in names for item in lista]
    print(names)

    # elimino los nombres en el texto
    for a in names:
        if a in words:
            words.remove(a)
    for word in words:
        print(word)
    if len(words)==0:
        print("tu texto no contiene palabras")
    elif len(words)==1:
        print("En el texto hay",len(words),"sola palabra")
    else:
        print("En el texto hay",len(words),"palabras distintas")

un ejemplo: en esta parte:

# lo analizo y cambio los signos de puntuacion por espacios
    text = "".join(letter if letter not in punct_signs and letter.isalpha()==True else " " for letter in text)
    # elimino todas las palabras repetidas y guardo todas las palabras distintas en una lista ordenado alfabeticamente
    words = sorted(set(text.lower().split()))

Yo hubiese usado un for para que analice letra por letra y reemplace todos los signos de puntuación y luego ir separandolos por espacio, sin embargo en una pregunta anterior que había hecho me mostraron esta forma mucho más sencilla usando las funciones "".join(), split() y set()

cualquier consejo ayuda a mi aprendizaje

6
  • acabo de añadir el codigo del programa, eso era solo un fragmento a modo de ejemplo
    – Mu-coder
    el 10 jul. 2019 a las 23:43
  • El bucle while es infinito, no hay manera se salir del bucle, ademas de que según veo, dicho bucle no tiene propósito alguno, corrigelo. el 11 jul. 2019 a las 0:03
  • ese ciclo es simplemente para que siga el programa siempre, eso esta hecho a proposito
    – Mu-coder
    el 11 jul. 2019 a las 0:06
  • Y como quieres que siga el programa? Solo ejecuto el código y me pide que introduzca el texto infinitamente, no tiene sentido. el 11 jul. 2019 a las 0:20
  • perdón, me falto una identación a todo el código
    – Mu-coder
    el 11 jul. 2019 a las 0:24

2 respuestas 2

4

Al margen del debate sobre si usar comprensiones de listas o bucles for tradicionales, hay varios aspectos que pueden mejorar la eficiencia del código, tanto desde el punto de vista del tiempo de ejecución, como de la legibilidad.

El filtrado de caracteres válidos

Por ejemplo, la línea que mencionas:

text = "".join(letter if letter not in punct_signs and letter.isalpha()==True else " " for letter in text)

Su objetivo es tomar el texto introducido y reemplazar todo lo que sean espacios, dígitos o signos de puntuación por espacios, dejando "pasar" sin modificaciones sólo las letras. Pero hay una redundancia, ya que si la letra está en punct_signs (que has definido como la suma de los dígitos string.digits más los signos de puntuación string.punctuation), entonces ya no será letra, y por tanto letter.isalpha() daría False. Y viceversa, si es una letra alfabética, letter.isalpha() será cierta, mientras que letter in punct_signs será falsa. Sólo puede ser cierta una de las dos, por lo que de algún modo estás comprobando dos veces la misma condición.

En definitiva, que basta con mirar si es alfabética, y en ese caso añadirla y en caso contrario cambiarla por espacio. Además tampoco necesitas mirar si letter.isalpha()==True, bata mirar si letter.isalpha(). Quedaría así:

text = "".join(letter if letter.isalpha() else " " for letter in text)

Esto ha ganado mucho en legibilidad y también en eficiencia de ejecución al testear menos condiciones. Si de todas formas quieres verlo como un bucle normal, sería así:

letras_procesadas = []  # Aqui se irán guardando letras y espacios ya procesados
for letter in text:
   if letter.isalpha():
      letras_procesadas.append(letter)
   else:
      letras_procesadas.append(" ")
text = "".join(letras_procesadas)

Es discutible si esta forma es más legible o no que la primera. A un programador principiante le parecerá más legible porque usa primitivas más básicas y bien conocidas, como bucles y condicionales. La otra forma en cambio requiere conocer características "avanzadas" como expresiones generadoras (lo de (expr for var in lista)) y el operador ternario (lo de expr if condicion else otra_expr). Pero si las conoces la expresión es perfectamente legible, y ocupa una sola línea de código, reduciendo su complejidad ciclomática.

Además, desde el punto de vista del tiempo de ejecución la forma con exporesión generadora es ligeramente más rápida (porque el compilador de python lo convierte a un bytecode más optimizado).

La función evaluate(name)

El punto en que se define

¿Por qué defines esa función dentro del bucle while? Esto plantea algunos problemas:

  1. Interrumpe en el lector la lectura del programa. Tiene que saltarse el trozo donde se define esta función, para ver cómo sigue el código. Lo hace más difícil de leer.
  2. Aumenta la indentación del código, un signo de mayor complejidad que influye en la mantenibilidad del código
  3. Obligas al intérprete a volver a definir esa función cada vez que pasa por una nueva iteración del while, lo cual es una ligera ineficiencia en tiempo de ejecución.

Si sacas esa función de donde está y la pones al principio del programa, se resuelven estos problemas.

Su cometido

A priori no está claro qué hace esa función y su nombre no ayuda mucho. Elegir buenos nombres para las variables y las funciones es algo que no influye en el tiempo de ejecución, pero sí puede ayudar mucho a mejorar la legibilidad y por tanto influir en el tiempo de desarrollo y de mantenimiento.

Tras inspeccionar lo que hace la función creo que está intentando demasiadas cosas. Es mejor que las funciones se dediquen a una sola cosa para que sean más fáciles de mantener. Tu función se ocupa de cuatro cosas (como se deduce también por los cuatro casos que retorna):

  1. Comprobar si el nombre está repetido, y retornar 1 en ese caso.
  2. Comprobar si la palabra introducida es "listo" y retornar 2 en ese cao.
  3. Comprobar si la palabra es válida (que contenga sólo letras y apóstrofes)
  4. Comprobar si está en la lista words y si no está retornar 4 (¿y si está? No retornas nada explícitamente en este caso)

Es difícil buscar un nombre apropiado para una función que hace tantas cosas y tan diversas, y esto es un síntoma de que esta función no debería existir. Todo sería más fácil si separas cometidos y cada objetivo se lo encargas a una función diferente:

  1. Repticiones de nombres. En realidad no necesitamos una función nueva para esto. Se podría resolver si nombres, en vez de ser una lista, fuese un set(), ya que este tipo de contenedor no almacena valores repetidos.
  2. Lo de comparar con "listo" parece un extraño cometido para esta función, pues ¿y si en vez de "listo" quieres más adelante que la palabra "mágica" para terminar sea FIN o END, etc.? Tendrías que alterar esta función. Creo que esta comprobación pertenece al bucle en que se van leyendo palabras y no a una función, además es una mera comparación, no hace falta escribir una función nueva para esto.
  3. Para ver si el nombre está formado por caracteres válidos, escribiríamos una nueva función que se llame por ejemplo is_valid_name(). Recibe como parámetro el name y retorna True si es válido o False si no. Un solo cometido claro.
  4. Para ver si el nombre está en words no hace falta escribir una función, pues es una mera comprobación de name in words.

Así pues todo se reduce a escribir una sola función nueva, que haga la validación de la palabra. Copiando tal cual parte de tu código, sería así:

def is_valid_name(name):
    for a in name:
        if a.isdigit():
            return False
        elif a.isalpha()==False:
            for i in a:
                if i in punct_signs and i!="'":
                    return False
    return True

Pero esto se puede simplificar mucho. Si name es una cadena, entonces a será cada letra de la cadena, por lo que el bucle for i in a no tiene sentido. Por otro lado al final se trata de verificar que todas las letras sean alfabéticas, o el apóstrofe. Esto podemos verlo de otra forma: si quito el apóstrofe, las restantes deben ser alfabéticas.

Quitar el apóstrofe es fácil: name.replace("'", "") y ver si el resultado es alfabético también es fácil pues basta hacer .isalpha() sobre el resultado, ya que isalpha() comprueba si todas las letras de la cadena son alfabéticas (no hay por qué usarlo con una sola letra).

Así pues, la función se reduce a una línea:

def is_valid_name(name):
    return name.replace("'", "").isalpha()

Fíjate que no tengo por qué mirar si el resultado es True para retornar True en ese caso o False en caso contrario. Basta retornar el resultado que ya será True o False.

El input

La línea en que lees los nombres de personas, lugares o marcas, haces input().split(), lo que da siempre como resultado una lista (si el usuario puso varias palabras separadas por espacios tendrás varios elementos, si escribió una sola palabra, tienes un solo elemento, pero una lista de todas formas). Ya que la variable en la que lees ese resultado la has llamado name, el nombre es engañoso, pues no es un nombre, sino una lista de ellos. Deberías iterar por esa lista, cosa que en principio no haces, salvo dentro de la antigua función evaluate() (ahora entiendo por qué tenías un segundo bucle for i in a, la función no recibía una cadena, sino una lista de ellas).

Debido a que name es una lista en tu caso, cuando haces names.append(name) estás metiendo una lista anidada dentro de la otra. Y es por eso que luego necesitas hacer esta línea para "aplanar" la lista:

names= [item for lista in names for item in lista]

Todo esto es muy confuso. Todo sería mucho más sencillo si fuerzas al usuario a meter una sola palabra de cada vez, que además sospecho que esta era tu intención, con lo que el .split() tras el input(), sobraría, y el "aplanado" de la lista también.

Lo que sí deberías haber hecho es pasar el resultado de input() a .lower(), pues así tienes almacenada la lista words. Si el usuario pone un nombre en mayúscula no sería encontrado.

Algunos trucos más

En el contexto de una expresión booleana puedes usar cosas que no son booleanos, y tienen el significado que cabría esperar. Si por ejemplo tienes un if lista:, eso será cierto si la lista tiene elementos y falso si no. Así que no necesitas hacer if len(lista)!=0: por ejemplo. Esto también hace el código más compacto y legible,

A la hora de imprimir cosas deberías usar str.format(), por ejemplo:

    print("En el texto hay {} palabras distintas".format(len(words)))

o si tienes python 3.7+ puedes usar las f-strings.

Código final

Moviendo la función que acabamos de programar al principio del programa, y haciendo uso del resto de técnicas antes descritas, el código podría quedar así:

def is_valid_name(name):
    return name.replace("'", "").isalpha()

while True:
    # solicito el texto
    text = input("Introduce tu texto: \n")
    # lo analizo y cambio los signos de puntuacion por espacios
    text = "".join(letter if letter.isalpha() else " " for letter in text)
    # elimino todas las palabras repetidas y guardo todas las palabras distintas en una lista ordenado alfabeticamente
    words = sorted(set(text.lower().split()))

    # solicito los nombres en el texto
    print("Introduce los nombres de personas, lugares o marcas presentes en tu texto: \n(escribe 'listo' cuando termines de escribir los nombres)")
    found_names = set()  # Nombres diferentes encontrados hasta ahora
    while True:
        # tomo el nombre
        name = input().lower()
        # descarto nombres repetidos
        if name in found_names:
            print("el nombre ya se ha introducido")
        # Compruebo si hay que salir del bucle
        elif name == "listo":
            break
        elif not is_valid_name(name):
            print("el nombre introducido no es valido")
        elif name not in words:
            print("el nombre no esta en el texto")
            continue
        # si pasa todas las pruebas, lo añade a la lista de nombres
        else:
            found_names.add(name)
    print(found_names)

    # elimino los nombres en el texto
    for a in found_names:
        if a in words:
            words.remove(a)
    for word in words:
        print(word)
    if not words:
        print("tu texto no contiene palabras")
    elif len(words)==1:
        print("En el texto hay una sola palabra")
    else:
        print("En el texto hay {} palabras distintas".format(len(words)))

Mejoras adicionales

Para reducir el nivel de indentación, y la complejidad del "programa principal", puedes escribir funciones de apoyo. Por ejemplo el bucle que pide datos al usuario y va mirando si están o no en la lista de palabras podría ser parte de una función, que reciba como parámetro la lista word y devuelva como resultado las que ha encontrado (found_names). La parte final que muestra los resultados podría ser otra función.

Al extraer código a funciones simplificas el programa que hacía uso de ese código, y disminuyes los niveles de indentación necesarios, que siempre mejora la legibilidad. Es importante elegir buenos nombres para esas funciones, de modo que leyendo sólo el programa principal tengas ya una idea clara de qué harán.

Quedaría así:

def is_valid_name(name):
    return name.replace("'", "").isalpha()


def input_names_and_find_them(words):
    found_names = set()  # Nombres diferentes encontrados hasta ahora
    # solicito los nombres en el texto
    print("Introduce los nombres de personas, lugares o marcas presentes en tu texto: \n(escribe 'listo' cuando termines de escribir los nombres)")
    while True:
        # tomo el nombre
        name = input().lower()
        # descarto nombres repetidos
        if name in found_names:
            print("el nombre ya se ha introducido")
        # Compruebo si hay que salir del bucle
        elif name == "listo":
            break
        elif not is_valid_name(name):
            print("el nombre introducido no es valido")
        elif name not in words:
            print("el nombre no esta en el texto")
            continue
        # si pasa todas las pruebas, lo añade a la lista de nombres
        else:
            found_names.add(name)
    return found_names


def print_results(words):
    for word in words:
        print(word)
    if not words:
        print("tu texto no contiene palabras")
    elif len(words)==1:
        print("En el texto hay una sola palabra")
    else:
        print("En el texto hay {} palabras distintas".format(len(words)))


# Programa principal
while True:
    # solicito el texto
    text = input("Introduce tu texto: \n")
    # lo analizo y cambio los signos de puntuacion por espacios
    text = "".join(letter if letter.isalpha() else " " for letter in text)
    # elimino todas las palabras repetidas y guardo todas las palabras distintas en una lista ordenado alfabeticamente
    words = sorted(set(text.lower().split()))

    found = input_names_and_find_them(words)
    print(found)

    # elimino los nombres en el texto
    for a in found:
        if a in words:
            words.remove(a)

    print_results(words)

El nombre de la función input_names_and_find_them() es un síntoma de que esa función no tiene una única responsabilidad. Podría plantearse si merecería la pena separarlo en dos funciones.

1
  • muchisimas gracias por tan basta explicacion, voy a reveer mi programa y aplicar lo que dijiste
    – Mu-coder
    el 11 jul. 2019 a las 19:26
-1

Veo que te gusta usar las estructuras de control rápidas dentro de las listas, Por ejemplo algo como [elemento for iteración if condición] Eso es bueno para programadores avanzado pero dificulta la lectura.

Una recomendación es que uses los f string, aquí algunos ejemplos:

S=10
print(f”la cantidad de monedas que tengo es {S}.”
M,cad=15,hola
print(f”Necesito {cad*(M-S)} palabras para {M-S**0} repeticiones”)
7
  • como se llama esa manera de imprimir las cosas? usando ´f´ ya que entiendo poco esa manera y me gustaria leer al respecto.
    – Mu-coder
    el 11 jul. 2019 a las 0:20
  • en la parte en la que aplano la lista: names= [item for lista in names for item in lista] no entiendo como funciona, lo encontré en esta pag. y funciono pero no se como funciona
    – Mu-coder
    el 11 jul. 2019 a las 0:22
  • Puedes ver la documentación de Python sobre la función print, ahí detalla el uso de los f-string. Básicamente permite evaluar operaciones dentro del string a imprimir.
    – L F
    el 11 jul. 2019 a las 0:23
  • 1
    Te recomiendo esta pagina que explica como funciona los f-strings: realpython.com/python-f-strings el 11 jul. 2019 a las 0:24
  • 1
    @Lucho K. En esta pregunta hay una explicación completa y extensa de compresión de listas: es.stackoverflow.com/questions/254468/… el 11 jul. 2019 a las 0:27

¿No es la respuesta que buscas? Examina otras preguntas con la etiqueta o formula tu propia pregunta.