Duda Code Smell: Misplaced Method

[marcosmyh]

Buenas días, ayer al realizar una refactorización de la entrega de QMP me encontré con un posible code smell: Misplaced Method. La primera versión tenía la siguiente forma:

abstract class Propuesta {
  Prenda prenda;
   
  public void modificarRopero(Ropero ropero){
     guardarEstadoRopero(ropero);
     aplicarModificacion(ropero);
  } 

  public abstract void aplicarModificacion(Ropero ropero);
}

class InsertarPrenda extends Propuesta {
  public void aplicarModificacion(Ropero ropero){
     ropero.agregarPrenda(ropero);
  }
}

class Ropero {
  private List<Propuesta> propuestas;
  private PropuestaAplicadas propuestasAplicadas; //Esta clase tiene la lista con las propuestas aplicadas.
  private List<Prenda> prendas;
  private Usuario usuario;

  public void aplicarPropuestasAprobadas(){
    propuestasAprobadas().forEach(propuesta -> aplicarPropuesta(propuesta));
  }

  public void aplicarPropuesta(Propuesta propuesta){
    propuesta.modificarRopero(this);
    propuestasAplicadas.agregarPropuesta(propuesta);
  }
}

Luego de advertir el posible code smell:

abstract class Propuesta {
  Prenda prenda;
   
  public void modificarRopero(Ropero ropero){
     guardarEstadoRopero(ropero);
     aplicarModificacion(ropero);
     ropero.agregarPropuestaAplicada(this);
  } 

  public abstract void aplicarModificacion(Ropero ropero);
}

class InsertarPrenda extends Propuesta {
  public void aplicarModificacion(Ropero ropero){
     ropero.agregarPrenda(ropero);
  }
}

class Ropero {
  private List<Propuesta> propuestas;
  private PropuestaAplicadas propuestasAplicadas; //Esta clase tiene la lista con las propuestas aplicadas.
  private List<Prenda> prendas;
  private Usuario usuario;

  public void aplicarPropuestasAprobadas(){
    propuestasAprobadas().forEach(propuesta -> propuesta.modificarRopero(this));
  }

  public void agregarPropuestaAplicada(Propuesta propuesta){
    propuestasAplicadas.agregarPropuesta(propuesta);
  }
}

¿Con cuál debería quedarme? Para mí, la segunda alternativa es más fácil de entender.

[natoszme]

Holi!
No sé si lo encajaría en un misplaced method porque aplicarPropuesta sí está afectando el estado interno del Ropero.
Si bien ya hay cierta bidireccionalidad en el envío de mensajes entre Ropero y Propuesta por ejemplo en guardarEstadoRopero(ropero) (o eso supongo pues miré tu repo pero no encontré su implementación), me quedaría con la primera porque estás pasando por la Propuesta para decirle al Ropero que la guarde como aplicada cuando el Ropero ya podría hacerlo (o sea, la lógica que tenés en aplicarPropuesta)

Avisame si se entendió y si coincidís en la elección :)

[marcosmyh]

Se entendió perfectamente y coincido con la elección 👍 Que el ropero aplique las propuestas que tiene aprobadas y se encargue de "registrarlas" es más apropiado.

[natoszme]

@marcosmyh lo marcás como Answered? :):)