Comentarios y sugerencias

[flbulgarelli]

¡Buenas!

Te dejo algunos comentarios sobre los code smells que identificaste:

5. Duplicated Code: [Movimiento. Método isDeposito()]:

No hace falta tener los dos métodos isDepósito() y isExtracción(). Se puede hacer un return !isDeposito() (igualmente no es demasiada repetición, puede ayudar tener los 2 para más declaratividad).

Ojo, podemos discutir si isDeposito o isExtraccion aportan o no a la solución, pero no hay lógica repetida allí. Duplicated Code se trata justamente de expresiones, métodos, inicializaciones de variables, clases, etc que tienen copias del código o incluso porciones de código muy similar. Sin embargo, acá no se observa nada de eso:

https://github.com/dds-utn/dds-monedero-java8/blob/2639ac6f7c067cece91266f3b21aeaa55851d704/src/main/java/dds/monedero/model/Movimiento.java#L39-L45

Notá que se trata de un getter y un método calculado que es la negación del otro. No tienen nada más en común que el uso de una misma variable de instancia. En general, es difícil hablar de duplicated code en métodos tan cortos, porque no hay ni siquiera una expresión en común más allá de referenciar una misma variable.

6. Misplaced Method: [Movimiento. Método agregateA(Cuenta)]:

Un Movimiento no tiene porque saber agregarse a una cuenta en particular. Es la cuenta quien tiene que agregar un Movimiento determinado basado en la fecha, monto y tipo de operación.

La discusión si conceptualmente le corresponde a un método estar en una clase o en otra sin suficiente contexto o justificación es bizantina. ¿Por qué un movimiento no tiene por qué saber agregarse? ¿De qué forma la definición de movimiento va en contra de eso? Podríamos discutirlo largo y tendido y no llegar a acuerdo.

Por el contrario, resulta siempre más productivo encarar la discusión de asignación de responsabilidades en base a tres ejes objetivos y taxativos:

1. ¿El objeto tiene el estado (o parte del estado) necesario para implementar ese método?
2. ¿El objeto tiene comportamiento (o parte del comportamiento) necesario para implementar ese método?
3. ¿El objeto introduce de forma total o parcial reglas polimórficas en la implementación de ese método?

Si alguna de estas tres preguntas es verdadera, entonces hay motivos para asignar la responsabilidad al objeto correspondiente. Sin embargo, el método agregateA (más allá de que pensemos filosóficamente como su responsabilidad) cumple las tres condiciones:

https://github.com/dds-utn/dds-monedero-java8/blob/2639ac6f7c067cece91266f3b21aeaa55851d704/src/main/java/dds/monedero/model/Movimiento.java#L47-L58

1. Provee fecha, monto, esDeposito
2. Emplea calcularValor
3. Y este último método está para permitir justamente una noción polimórfica (oculta porque hay un type test)

Por todo esto, no es descabellado que al menos parte del comportamiento de agregateA siga estando en esta clase.

En oposición a las cuestiones ontológicas y discusiones semánticas, los code smells se manifiestan en relaciones estructurales entre objetos, clases, métodos, etc. Mirá estos ejemplos de missplaced method:

• https://devcards.io/misplaced-method
• https://sites.google.com/site/utndesign/cursos/cursadas-anteriores/miercoles-a-la-noche-2012/seguimientoclasesmienoche2012/refactoring

En ambos ejemplos, el missplaced method (y los code smells, en general) se pueden detectar con independencia del dominio, porque hablan de propiedades taxativas del código que no se cumplen más allá de lo que éste represente (largo de un método o una clase, porciones de código repetido, método que no se implementa polimórfica, no envía mensajes al recepetos ni hace participar su estado, etc)

[flbulgarelli]

Por otro lado, te dejo unas sugerencias sobre el código:

1. Cuenta.setMovimientos se debería eliminar, ya que nunca se usa y expone estado mutable innecesariamente.
2. Un detalle de implementación: en Java cuando una interfaz implementa compareTo en general implementa alguna noción de equivalencia, y los BigDecimal's no son la excepción. Por eso es que en lugar de usar compareTo acá .... https://github.com/FrancoPaesani/dds-monedero-java8/blob/b55f4a46acafa4dbc69f395292e9835bd0cb48ed/src/test/java/dds/monedero/model/MonederoTest.java#L38-L40 ... podrías usar un equals común y corriente o incluso un assertEquals.
3. Acá ... https://github.com/FrancoPaesani/dds-monedero-java8/blob/b55f4a46acafa4dbc69f395292e9835bd0cb48ed/src/main/java/dds/monedero/model/Cuenta.java#L34-L51 ... sigue habiendo código duplicado. Notá que los dos métodos inician y terminan con las mismas líneas.

¡Saludos!