Skip to content

Comentarios QMP1 #1

@ggallici

Description

@ggallici

Buenas, como va? Antes que nada, perdón por la demora para el feedback.
Te voy marcando algunas cositas que fui viendo:

  • No deberías usar el Main para testear tu dominio, para eso tenemos los tests :P
  • El requerimiento general del enunciado no habla de combinar distintas prendas en un Atuendo sino de la carga/validación de cada una de las mismas.
  • Si bien usar enums para modelar los colores es un solución superadora a utilizar strings, puede llevar a una clasificación un poco arbitraria (ROSA, ROSA_CLARITO, ROSA_MUY_CLARITO, etc) y, además, nos acota demasiado los posibles valores de colores del sistema. Una buena alternativa sería modelar los colores como una combinación RGB que es un poco mas standard y nos da un plus de flexibilidad.
  • No está mal tener comportamiento extra para facilitar el testeo tu código (de hecho, es una buena práctica), pero sí hacerlo de forma incorrecta. Los mensajes getCategoriay getPrenda no aportan nada al dominio y tampoco ayudan a testear ya que un string es poco práctico para hacerle aserciones.
  • El colorSecundario no está siendo opcional en tu solución, sino que siempre es null. Una alternativa simple para cumplir con este requerimiento sería recibirlo por constructor pero nunca validar si es nulo o no.
  • Por otro lado, sobre el diagrama de clases, no hace falta que siempre diagrames todos los mensajes. Con que estén los más relevantes alcanza.

Igualmente, tu solución está muy muy bien 🥳.
Saludos y nos vemos el Viernes!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions