fjgallardo00 / Akinah

Repositorio donde se desarrollan las prácticas de IV 2022/2023
GNU General Public License v3.0
0 stars 1 forks source link

Objetivo 2 [IV-22-23] #14

Open JoseCarlosPPK opened 1 year ago

JoseCarlosPPK commented 1 year ago

Sobre la estructura del repositorio

Sobre el análisis del problema

Sobre la planificación y la programación

Close #9 porque está mal planteado y no se va a seguir con su desarrollo. Close #11 porque no se va a seguir con su desarrollo.

JoseCarlosPPK commented 1 year ago

En estos dos últimos commits la he liado un poco. El commit d6a3b8b lo quería subir sin una entrada para entidad en el fichero iv.yaml y subí un fichero desactualizado

pedrooot commented 1 year ago

Como ha comentado @JJ, los libros tienen más atributos de los que estás creando en libro.js (título...). Tampoco me queda muy claro el tema de la puntuación ya que esta se asigna dependiendo de los gustos de cada usuario y la implementación actual de la misma toma la puntuación como algo general sin tener en cuenta el usuario (Para cada libro, la puntuación del mismo es distinta dependiendo de los gustos del usuario).
Intenta ir revisando estas cosas y vamos viendo 😬

JoseCarlosPPK commented 1 year ago

Listo para revisión @fjgallardo00 !

JoseCarlosPPK commented 1 year ago

Perdón por cerrar el PR, ha sido sin querer. Me he confundido con cancelar el comentario que iba a escribir

JoseCarlosPPK commented 1 year ago

El resumen de los cambios:

  1. Los géneros se han modelado como una clase que como atributo de clase tiene el conjunto de géneros global y cada instancia será un subconjunto de este.
  2. Se han modelado los usuarios. Cada usuario encapsula sus gustos, que son objetos de la clase ConjuntoGeneros.
  3. Se ha eliminado la clase que relacionaba la puntuación con los libros. Como bien dijo @pedromarting3 , el modelo anterior representaba una puntuación global para todos los usuarios para un mismo libro. Una vez modelados los usuarios, para solucionar el problema comentado por @pedromarting3, en vez de tener la pareja (libro, puntuación) se podría tener la terna (usuario, libro, puntuación), pero la idea de esta clase era ayudar a la lógica de negocio cuando tuviera que ordenar los libros con mayores puntuaciones. Es aquí cuando no me queda claro si esta clase debería formar parte del modelo y por eso la he quitado, pensando que es algo más interno de la lógica de negocio
JoseCarlosPPK commented 1 year ago

Con esto estaría listo para revisión @fjgallardo00

fjgallardo00 commented 1 year ago

@JoseCarlosJC el archivo de iv.yaml le falta un retorno de carro. No te pasarán los tests de JJ si no se lo pones. Por lo demás lo veo todo bien

JoseCarlosPPK commented 1 year ago

Le he añadido esa nueva línea al final aunque no se muestra aquí una tercera línea

JoseCarlosPPK commented 1 year ago

Listo para revisión @JJ . No puedo volver a pasar los tests porque me aprobó el PR y no debería abrir otro para el mismo objetivo según las normas

JoseCarlosPPK commented 1 year ago

Listo para revisión @fjgallardo00 @JJ! He quitado el setter y el código de asginación al atributo solo lo ejecuta el constructor. Además como mejora (pienso), ningún objeto de la clase puede modificar el atributo de clase GENEROS. Para los libros he quitado el atributo descripcion que no se menciona en ningún issue y no es necesario para el PMV.

El issue #12 he reformulado el título para que exprese un problema, no sé si el error estaba ahí, en la expresión. El problema es que se han de modelar los libros porque la lógica de negocio los usa para sus cálculos y finalmente como recomendaciones (esto está expresado en el issue)

JJ commented 1 year ago

Listo para revisión @fjgallardo00 @JJ! He quitado el setter y el código de asginación al atributo solo lo ejecuta el constructor. Además como mejora (pienso), ningún objeto de la clase puede modificar el atributo de clase GENEROS. Para los libros he quitado el atributo descripcion que no se menciona en ningún issue y no es necesario para el PMV.

El issue #12 he reformulado el título para que exprese un problema, no sé si el error estaba ahí, en la expresión. El problema es que se han de modelar los libros porque la lógica de negocio los usa para sus cálculos y finalmente como recomendaciones (esto está expresado en el issue)

Repetidamente os he dicho que reformular un issue es una mala práctica. La buena práctica es cerrarlo cuando se borra el código que se ha creado con él y crear un nuevo issue.

JoseCarlosPPK commented 1 year ago

Listo para revisión @fjgallardo00 @JJ! He quitado el setter y el código de asginación al atributo solo lo ejecuta el constructor. Además como mejora (pienso), ningún objeto de la clase puede modificar el atributo de clase GENEROS. Para los libros he quitado el atributo descripcion que no se menciona en ningún issue y no es necesario para el PMV. El issue #12 he reformulado el título para que exprese un problema, no sé si el error estaba ahí, en la expresión. El problema es que se han de modelar los libros porque la lógica de negocio los usa para sus cálculos y finalmente como recomendaciones (esto está expresado en el issue)

Repetidamente os he dicho que reformular un issue es una mala práctica. La buena práctica es cerrarlo cuando se borra el código que se ha creado con él y crear un nuevo issue.

Pues tengo un problema. Si tuviera que crear otro issue sería muy parecido al que ya está. Pensaba que el error estaba en cómo había formulado o expresado el issue (el título). Me gustaría saber el porqué no representa un problema el issue #12

JJ commented 1 year ago

No he dicho que no represente un problema. He dicho que reescribir un issue es una mala práctica. Hay que cerrarlo, borrar el código y hacer uno nuevo.

JoseCarlosPPK commented 1 year ago

Otra pregunta @JJ es si tengo que tener un issue general, que represente al objetivo por así decirlo para añadir el fichero iv.yaml . O tal vez para este no haga falta

JJ commented 1 year ago

TL;DR: siempre que se aporte código al repositorio, hay que hacerlo siguiendo un issue.

Otra pregunta @JJ es si tengo que tener un issue general, que represente al objetivo por así decirlo para añadir el fichero iv.yaml . O tal vez para este no haga falta

Esa no es la pregunta. La pregunta es si el issue plantea un problema. Los issues siempre tienen que plantear un problema.

También la pregunta es "si pongo esto así te quedarás satisfecho". No, no me quedaré satisfecho. Los issues se crean no porque a mi me guste o me haya dado por ahí, y no se plantean como un problema para echar para atrás todo lo que no me parezca un problema. Se plantean porque la ingeniería es resolver problemas (dicho el primer día de clase) y la única forma de saber si realmente se ha resuelto un problema es plantear un issue, escribir código para resolverlo, y escribir un test que compruebe que efectivamente el código resuelve el problema.

Por otro lado, la pregunta es "si hago esto así, si estará bien o no". Y esto también os lo respondí el primer día. La asignatura, y la ingeniería en general, te tiene que decir dos cosas: qué tengo que hacer ahora, y si lo que hago está bien. Para lo primero, está la progresión de los objetivos; para lo segundo, la asignatura tiene las revisiones de código. Así que hasta que no escribas issue, escribas el código correspondiente, y hagas el test (o en este objetivo aprobación por @fjgallardo00 y por mi parte) o se revise, no sabré si está bien o no.

Entiendo que queráis optimizar vuestro tiempo y no hacer cosas que están mal. Pero debéis de entender que para no hacer cosas que están mal tenéis que haber seguido las directrices y metodologías que están en clase, y la metodología de dar clase implica que si no son correctas o no contribuyen al objetivo, se revisan y se solicita que se cambien. Para optimizar mi tiempo, desde luego, lo más adecuado sería que intentárais hacer las cosas tras mirar los errores frecuentes y las explicaciones que se han hecho en clase, en este caso sobre el concepto de issue. Pero es legítimo que hagáis lo que veáis y yo tenga que revisarlo; aparte, es mi obligación hacerlo y es como aporto valor a vuestro trabajo.