andrleo0
commented
4 years ago profesor @urielkelman quería preguntarle que cuando se refiere a Los prints que escriben cosas en minúsculas son desprolijos (ejemplo línea 187), no se muy bien si se refiere a la apertura de archivos con el open, me gustaría saber bien así aplico la corrección lo mejor posible.Gracias
agrego que voy a realizar un commit del modulo de carga con los cambios de las correcciones que llevo hasta el momento
urielkelman
El nombre la función solicitar_archivo está mal elegida, ya que no están solicitando nada, lo están cargando directamente. También tienen un print en el main antes de invocar al módulo que ensucia la salida. Tampoco creo que esta función sea necesaria, solamente realiza un open de la ruta harcodeada... podría no estar.
Los prints que escriben cosas en minúsculas son desprolijos (ejemplo línea 187).
La función validación función inicial se puede escribir como:
return linea_modulo.startsWith('def ')
Es decir, podemos devolver directamente el resultado de evaluar la expresión, sin considerar variables intermedias.
El siguiente pedazo de código tiene 3 ifs anidados; que deberían ser expresados en una sola condición que utilice la conjunción mediante el operador and.
if linea_modulo[0:4] != ' ': if linea_modulo != '\n': if not linea_modulo.startswith('def ') :
Las funciones "calculo_comentario_multiples" y "lineas_restantes" no tienen return. Si bien no se puede afirmar que esto esté directamente mal, ambas funciones trabajan sobre el diccionario de funciones (que podemos modificar dentro de la función ya que diccionario es un tipo de dato mutable). Es una buena práctica devolver el diccionario y asignarlo al retorno de la función en la línea en la que se invoca la función para denotar justamente que la función opera sobre el diccionario.
La función validacion_funcion_final recibe dos parámetros que no usa, ensuciando el código tanto de la firma de la función como su invocación.
Hubiera estado bueno definir constantes al tope del archivo para poder seguir mejor los índices de la lista que tienen la información de la función. Podrían estar en mayúscula, por ejemplo:
INDICE_AUTOR = 2 -> de esta forma es más fácil identificar a qué indice corresponde determinada información. Otra forma, que se la recomiendo que tengan en cuenta pero no hace falta que lo arreglen, es utilizar una estructura de diccionarios y acceder con claves, siendo esto más legible que usar índices (esto lo charlamos en clase cuando di repaso para el parcial, no sé si recuerdan). Por ejemplo: funciones[nombre_funcion]["autor"] es bastante legible.
En la función principal del módulo, hay 3 whiles anidados. Si bien el código se entiende, quedó bastante corrido hacia la derecha, generando "código flecha". Capaz lo que se encuentra dentro del tercer while anidado podría estar dentro de una función aparte.
c_multiple -> !No ahorren caracteres! Si es descriptivo, sin exagerar, es mejor. Usar "comillas_multiples".
Dentro del tercer while anidado, nos encontramos con:
if nombre_funcion != '':
Esto sucede ya que ya hemos inicializado el nombre de la función en el diccionario, y se elige devolver comillas vacías en la invocación a la función crear_función justamente porque este nombre ya existe. Esto pasa cuando la línea que leemos no empieza con "def ", lo cual tiene sentido. Ahora bien, todas las invocaciones que nacen de invocar a "crear_funcion" no son necesarias, ya que si no tengo una función nueva, y sé que voy a devolver un string vacío, para qué la invoco?
La lógica que usaron los obliga a estar devolviendo strings vacíos, lo cual no es una buena práctica. Si hacían directamente:
if linea_modulo.startswith('def '): nombre_guardado = crear_funcion(linea_modulo,funciones,nombre_modulo,)
De esta forma se evitaban estar llamando funciones que pueden devolver strings vacíos, lo cual SI es una mala práctica. No tienen que seguir la lógica que les propongo, pero sí tienen que corregir el código de forma tal de no devolver strings vacíos.
Eso es todo de este módulo. Si no entienden alguna corrección, pueden preguntarme. Entiendo que esta parte la va a corregir Andrés. Como comentario positivo, me gustó la modularización y sobre el merge no tengo ninguna corrección. Podría extrapolar algunas de las correcciones que marqué antes, por ejemplo algunos nombres de variables que no me gustaron, o también por ejemplo que dentro de la función "validar_fin_líneas" pueden devolver el resultado de una expresión y no hace falta tener la variable validar. Pueden fijarse si encuentran más cosas a partir de las correcciones que les marqué y corregirlas.