5
votes

Fusion de code dupliqué utilisant différents objets

J'utilise deux appels API pour obtenir des données sur vehicleUtils en fonction de contentFilter. J'ai un code très similaire pour les deux (conducteurs et véhicules). Ce que j'ai essayé de faire est d'extraire le code en une seule méthode et d'appliquer le modèle de stratégie comme ils le suggèrent ici Méthodes de refactorisation , mais je n'ai pas pu comprendre comment l'implémenter. Est-ce que j'utilise une bonne approche ou y a-t-il une meilleure façon?

if (contentFilter.equalsIgnoreCase(Contentfilters.VEHICLES.toString())) {

  VuScores vuScores = new VuScores();
  List<VehicleVuScores> vehicleVuScoresList = new ArrayList<>();
  List<VehicleUtilization> vehicleUtilizations = RestClient.getVehicles(request).join().getVehicleUtilizations();


  if (Objects.nonNull(vehicleUtilizations)) {
    vehicleUtilizations.forEach(vehicleUtilization -> {
      vuScores.getVehicleVuScores().forEach(vehicleVuScore -> {

        vehicleVuScore.getScores().setTotal(vehicleUtilization.getFuelEfficiencyIndicators().getTotal().getValue());
        vehicleVuScore.getScores().setBraking(vehicleUtilization.getFuelEfficiencyIndicators().getGroupIndicators().get(3).getIndicators().get(0).getValue());
        vehicleVuScore.getScores().setCoasting(vehicleUtilization.getFuelEfficiencyIndicators().getGroupIndicators().get(3).getIndicators().get(1).getValue());
        vehicleVuScore.getScores().setIdling(vehicleUtilization.getFuelEfficiencyIndicators().getGroupIndicators().get(0).getIndicators().get(0).getValue());
        vehicleVuScore.getScores().setAnticipation(vehicleUtilization.getFuelEfficiencyIndicators().getGroupIndicators().get(3).getValue());
        vehicleVuScore.getScores().setEngineAndGearUtilization(vehicleUtilization.getFuelEfficiencyIndicators().getGroupIndicators().get(1).getValue());
        vehicleVuScore.getScores().setStandstill(vehicleUtilization.getFuelEfficiencyIndicators().getGroupIndicators().get(0).getValue());
        vehicleVuScore.getScores().setWithinEconomy(vehicleUtilization.getFuelEfficiencyIndicators().getGroupIndicators().get(1).getIndicators().get(7).getValue());
        vehicleVuScore.setAvgFuelConsumptionPer100Km(vehicleUtilization.getMeasures().getTotal().getAverageConsumption().getValue());
        vehicleVuScore.setAvgSpeedDrivingKmh(vehicleUtilization.getMeasures().getTotal().getAverageSpeed().getValue());
        vehicleVuScore.setEngineLoad(vehicleUtilization.getFuelEfficiencyIndicators().getGroupIndicators().get(1).getIndicators().get(1).getValue());
        vehicleVuScore.setTotalDistanceInKm(vehicleUtilization.getMeasures().getDriving().getDistance().getValue());
        vehicleVuScore.setTotalTime(Math.toIntExact(vehicleUtilization.getMeasures().getTotal().getTime().getValue()));

        vehicleVuScoresList.add(vehicleVuScore);
      });
    });
    vuScores.setVehicleVuScores(vehicleVuScoresList);
  }
  return CompletableFuture.completedFuture(vuScores);

} else if (contentFilter.equalsIgnoreCase(Contentfilters.DRIVERS.toString())) {

  VuScores vuScores = new VuScores();
  List<DriverVuScores> driverVuScoresList = new ArrayList<>();
  List<VehicleUtilization> vehicleUtilizations = RestClient.getDrivers(request).join().getVehicleUtilizations();


  if (Objects.nonNull(vehicleUtilizations)) {
    vehicleUtilizations.forEach(vehicleUtilization -> {
      vuScores.getDriverVuScores().forEach(driverVuScores -> {

        driverVuScores.getScores().setTotal(vehicleUtilization.getFuelEfficiencyIndicators().getTotal().getValue());
        driverVuScores.getScores().setBraking(vehicleUtilization.getFuelEfficiencyIndicators().getGroupIndicators().get(3).getIndicators().get(0).getValue());
        driverVuScores.getScores().setCoasting(vehicleUtilization.getFuelEfficiencyIndicators().getGroupIndicators().get(3).getIndicators().get(1).getValue());
        driverVuScores.getScores().setIdling(vehicleUtilization.getFuelEfficiencyIndicators().getGroupIndicators().get(0).getIndicators().get(0).getValue());
        driverVuScores.getScores().setAnticipation(vehicleUtilization.getFuelEfficiencyIndicators().getGroupIndicators().get(3).getValue());
        driverVuScores.getScores().setEngineAndGearUtilization(vehicleUtilization.getFuelEfficiencyIndicators().getGroupIndicators().get(1).getValue());
        driverVuScores.getScores().setStandstill(vehicleUtilization.getFuelEfficiencyIndicators().getGroupIndicators().get(0).getValue());
        driverVuScores.getScores().setWithinEconomy(vehicleUtilization.getFuelEfficiencyIndicators().getGroupIndicators().get(1).getIndicators().get(7).getValue());
        driverVuScores.setAvgFuelConsumptionPer100Km(vehicleUtilization.getMeasures().getTotal().getAverageConsumption().getValue());
        driverVuScores.setAvgSpeedDrivingKmh(vehicleUtilization.getMeasures().getTotal().getAverageSpeed().getValue());
        driverVuScores.setEngineLoad(vehicleUtilization.getFuelEfficiencyIndicators().getGroupIndicators().get(1).getIndicators().get(1).getValue());
        driverVuScores.setTotalDistanceInKm(vehicleUtilization.getMeasures().getDriving().getDistance().getValue());
        driverVuScores.setTotalTime(Math.toIntExact(vehicleUtilization.getMeasures().getTotal().getTime().getValue()));

        driverVuScoresList.add(driverVuScores);
      });
    });
    vuScores.setDriverVuScores(driverVuScoresList);
  }
  return CompletableFuture.completedFuture(vuScores);
}


3 commentaires

En regardant votre code, je dois me demander: est-ce que VehicleVuScores et DriverVuScores ont une superclasse ou une interface commune? Si c'est le cas, la seule différence que je vois (ad hoc) est l'initialisation de la List et l'appel à l'API REST. Le reste me ressemble à peu près. Le code dans les cas if-else se réduirait alors à deux lignes chacun, le reste du code étant en dehors des deux cas, éliminant toute duplication.


Encore une observation: avec un peu plus de refactoring, c'est-à-dire en déployant par exemple le modèle Builder , vous pourrait encore améliorer la lisibilité de votre méthode: VehicleVuScore vehicleVuScore = VehicleVuScore.builder (). from (vehicleUtilization) .build (); vehiculeVuScoreList.add (vehiculeVuScore);


Cette question appartient sur codereview.stackexchange.com .


3 Réponses :


1
votes

Utilisez une interface, implémentez-la dans les deux classes et utilisez cette interface aux deux endroits pour obtenir ou définir des valeurs. Puisque tous les noms de méthode sont identiques, l'interface doit contenir tous les getters et setters nécessaires. De cette façon, vous n'aurez pas à utiliser différentes classes.


0 commentaires

4
votes

Essayez de penser à une classe de base commune (abstraite), qui contient le code commun. Les classes réelles contiennent le code différent.

Vous n'avez alors pas besoin de travailler avec instanceof ou Contentfilters ou tout autre type de fonctionnalité de désactivation que vous utilisez. Vous pouvez simplement appeler les méthodes courantes, car votre fonction doit prendre la classe de base (abstraite). Cela supprime vraiment la duplication de code.


0 commentaires

1
votes

Donc, tout est pareil sauf

  • les types de DTO dans lesquels vous copiez les données (VehicleVuScores vs DriverVuScores)
  • la méthode RestClient appelée

Le principal défi est de partager le code qui invoque les setters. Nous avons besoin d'un moyen de faire référence à l'objet cible sans savoir s'il s'agit d'un VehicleVuScores ou d'un DriverVuScores. Nous pourrions le déclarer comme:

List<VuScoresBase> convertList(List<VehicleUtilization> vehicleUtilizations) {
  // iterate
     result.add(convert(vehicleUtilization));
}

mais comme Object ne déclare pas les setters, nous aurions des erreurs de compilation en essayant d'appeler les setters . Pour résoudre ce problème, nous pouvons déplacer la déclaration de ces getters et setters dans un type de base commun:

VuScoresBase convert(VehicleUtilization vehicleUtilization) {
   VuScoresBase vuScores = new VuScoreBase();
   // invoke setters
   return vuScores;
}

afin que nous puissions écrire:

convertList(vehicleUtilizations, driverVuScores, DriverVuScore::new);


0 commentaires