search

mnigliazzo / qmp-grupo10

0 stars 0 forks source link

Comentarios QMP5 #9

Open SofiCortes opened 3 years ago

SofiCortes commented 3 years ago

Buenas!

Les dejo comentarios sobre QMP5:

  1. En el diagrama de clases faltan los * en las relaciones de listas. Cuidado con este punto porque es un punto importante para la comunicación del diseño, sino a simple vista veo que un Usuario solo puede tener un guardarropas y una sola Sugerencia. Screen Shot 2021-06-18 at 21 07 03
  2. Ojo con usar el término Sugerencia, porque ya hablamos de este término en entregas anteriores y puede confundirse con esta entrega (en su código . Pueden ayudarse con lo que dice el enunciado, que habla de proponer
  3. https://github.com/mnigliazzo/qmp-grupo10/blob/479b39806d1dabf8346006bcfb1719337840750b/src/main/java/domain/prenda/Usuario.java#L12-L16 Es un missplaced method camuflado con una validación. Primero, no necesitamos esa validación, nuestro sistema no deberia tener la posibilidad de generar una propuesta sobre un guardarropas que no tiene el usuario. Al quitar esta validación, nos damos cuenta que el método no utiliza ningún atributo interno y podría moverse directamente al guardarropas.
  4. https://github.com/mnigliazzo/qmp-grupo10/blob/479b39806d1dabf8346006bcfb1719337840750b/src/main/java/domain/prenda/Guardarropa.java#L5-L11 No están inicializando las listas, con lo cual van a tener un NullPointerException cuando ejecuten el código. Las pueden inicializar en el constructor o al momento de declarar las variables.
  5. https://github.com/mnigliazzo/qmp-grupo10/blob/479b39806d1dabf8346006bcfb1719337840750b/src/main/java/domain/prenda/Guardarropa.java#L19-L23 y con la misma validación en los otros métodos: mismo comentario que (3), no necesitamos esa validación porque no debería ser posible aceptar/rechazar/deshacer una sugerencia que no esté en la lista.

Saludos!

mnigliazzo commented 3 years ago

Muchas Gracias @SofiCortes por la corrección. Lo tengo en cuenta para corregirlo.

Saludos