search

sashaestrella / QueMePongo

1 stars 0 forks source link

Correcciones y sugerencias #1

Open flbulgarelli opened 3 years ago

flbulgarelli commented 3 years ago

¡Buenas!

Te dejo nuestros comentarios:

  1. Primero, hiciste una solución que funciona (y tiene tests :clap:) aunque no era requerido. ¡Excelente trabajo!
  2. Cuando hacés tests de excepciones como acá... https://github.com/sashaestrella/QueMePongo/blob/954e65d7a31afe0e2dd766baec85492a67b909f7/src/test/java/test/QueMePongoTest.java#L30-L32 ... lo recomendable es probar contra excepciones más específicas que Exception, dado que si ese código fallara por otro motivo distinto de lo que querés probar (la validación), el mismo daría verde cuando en realidad querías que de rojo.
  3. Si bien no está mal, es temprano para subclasificar las tipos de prendas ... https://github.com/sashaestrella/QueMePongo/blob/954e65d7a31afe0e2dd766baec85492a67b909f7/src/main/java/main/Camisa.java#L8-L15 ... y probablemente puedas resolver el problema enumerándolas.
  4. Dado que éste comportamiento es propio del concepto general de color y no específico de un color particular... https://github.com/sashaestrella/QueMePongo/blob/954e65d7a31afe0e2dd766baec85492a67b909f7/src/main/java/main/Color.java#L19-L24 ... lo declararía como static.
  5. Si bien acá estamos en un constructor .... https://github.com/sashaestrella/QueMePongo/blob/954e65d7a31afe0e2dd766baec85492a67b909f7/src/main/java/main/Prenda.java#L13-L27 ... y si se lanza una excepción en cualquier lugar de ese código se aborta la construcción del objeto, lo recomendable es siempre validar primero y luego realizar todas las asignaciones, para no correr el riesgo de que se produzcan efectos indeseables antes de que la validación falle.
  6. Este segundo constructor ... https://github.com/sashaestrella/QueMePongo/blob/954e65d7a31afe0e2dd766baec85492a67b909f7/src/main/java/main/Prenda.java#L30-L42 ... repite lógica que está en el anterior. Para evitarlo, deberías delegar el primero en el segundo.
  7. En Java declarar que algo lanza una Exception a secas (mediante throws Exception) tiene un inconveniente: obliga a que arrastres esa declaración por todo tu código. Esto se conoce como excepciones chequeadas y recomendamos que no lo pongas en tu código.
flbulgarelli commented 3 years ago

Sobre el último tema, te recomiendo que mires estas secciones de los apuntes:

sashaestrella commented 3 years ago

Muchas gracias profe por la corrección y las explicaciones bien detalladas, voy a tener en cuenta todo eso y tratar de aplicarlo en la entrega de Macowins