Comprendre le principe de la responsabilité unique avec ce vrai code

Question

Donc, quand vous essayez de vous éduquer pour le principe de la responsabilité unique, vous rencontrerez probablement des définitions comme "Une méthode devrait faire une seule chose." et "Une classe devrait être juste une raison de changer". Et puis il y a toujours des tas d'exemples de jouets comme

class Dog { 
    String bark() { 
     return "Woof"; 
    } 
} 

Mais je trouve qu'il est très difficile d'appliquer ce principe dans l'entreprise de développement d'applications.

Nous avons une application où nous avons "Éléments du projet", "Activités" et "Employés". Un employé aura plusieurs activités et une activité peut avoir un élément de projet.

Certains employés sont affectés à tous les éléments du projet, d'autres seulement à un groupe. Un élément de projet peut être supprimé d'un employé et de nouveaux peuvent être affectés. L'employé peut seulement ajouter de nouvelles activités avec des éléments de projet qui lui sont assignés. Si aucun élément de projet n'est affecté, cela signifie que tous sont disponibles.

Un client de nous avait une exigence, ils voulaient voir une liste des éléments de projet favoris lors de l'ajout de nouvelles activités. Peu importe comment un employé fait la promotion d'un élément de projet, il s'agit d'une vue différente.

Les éléments favoris du projet doivent être classés par des règles intéressantes. Cela dépend du travail total calculé des activités où un élément de projet favori existe, il ne tient compte que des 60 derniers jours. Au moins, il est une exigence concrète ..

Il était de mon devoir de mettre en œuvre, et voici la mise en œuvre, j'ai:

public List<ProjectElement> getFavoriteProjectElementsSortedDescendingByTotalWorkHoursInLast60Days(Employee owner) { 
    // All Favorite Project Elements of Owner 
    final List<ProjectElement> favoriteProjectElementsOfOwner 
      = (List<ProjectElement>) (List<?>) favoriteBusinessObjectHelper.getAllFavoriteObjectsForBusinessObjectType(BusinessObjectType.PROJECT_ELEMENT.toInt()); 

    // Activities by Favorite Project Elements 
    final List<Activity> favoriteProjectElementActivitiesForLast60Days 
      = activityFinder.findByInProjectElementsForLastGivenDates(owner, favoriteProjectElementsOfOwner, 60); 

    // Create a Map with Favorite Project Element - Calculated Work 
    final Map<ProjectElement, Long> projectElementTotalWorkMap = new HashMap<ProjectElement, Long>(); 
    for (Activity activity : favoriteProjectElementActivitiesForLast60Days) { 
     final ProjectElement activityProjectElement = activity.getProjectElement(); 
     if (!projectElementTotalWorkMap.containsKey(activityProjectElement)) { 
      projectElementTotalWorkMap.put(activityProjectElement, 0L); 
     } 
     final long calculatedWork = activity.getCalculatedWork(); 
     final long totalCalculatedWork = projectElementTotalWorkMap.get(activityProjectElement) + calculatedWork; 
     projectElementTotalWorkMap.put(activityProjectElement, totalCalculatedWork); 
    } 

    // Sort the Map by value descending 
    final Map<ProjectElement, Long> sortedProjectElementTotalWorkMap 
      = InnboundSortTool.sortByValueDescending(projectElementTotalWorkMap); 

    // We do not want to show owners Favorite Project Element in the sidebar, if the Project Element is not available 
    // for Employee anymore.. See the comments at the end of the file. 
    final Set<ProjectElement> allowedProjectElementsForOwner = owner.getExplicitlyAssignedOnlyActiveProjectElements(); 

    final ArrayList<ProjectElement> projectElementsSorted = new ArrayList<ProjectElement>(); 
    for (ProjectElement projectElement : sortedProjectElementTotalWorkMap.keySet()) { 
     if (allowedProjectElementsForOwner.size() == 0) { // This means Employee does not have any restrictions, all Project Elements are available to him. 
      projectElementsSorted.add(projectElement); 
     } else { // If Employee has assigned Project Elements, we must check if the Favorite Project Element is assigned to him.. 
      if (allowedProjectElementsForOwner.contains(projectElement)) { 
       projectElementsSorted.add(projectElement); // If yes, add it to list, if no simply continue the loop without adding. 
      } 
     } 
     if (projectElementsSorted.size() == 20) { 
      break; // 20 is an arbitrary value, we do not want to show too many Favorite Project Elements in the UI.. Limit by 20. 
     } 
    } 

    return projectElementsSorted; 
} 

Whoa, qui est une grande méthode, mais il fait le travail terminé. Mais ça ne fait pas une chose, n'est-ce pas? Mais si chaque méthode ne fait qu'une chose, qui va faire le tout?

Ai-je introduire une classe d'aide et tout délégué à la cette classe et commencer à appeler:

final Map<ProjectElement, Long> projectElementTotalWorkMap = helper.CreateprojectElementTotalWorkMap(); 

helper.removeUnassignedFavoriteProjectElementsFromEmployee(); 

etc? Mais puis-je présenter un assistant à Helper lui-même? Où finit-il même? Quand je commence refactoring comme ça, je finis par avoir des méthodes extrêmement inutiles qui il suffit d'appeler d'autres méthodes, et cette classe ressemblera à ceci:

List<ProjectElement> favoritesList; 
favoritesList = helper.doThis(); 
favoritesList = helper.doThat(); 
favoritesList = helper.sort(); 
return favoritesList; 

Dois-je comprendre pas ce principe du tout? Je suppose que non, alors voici la question, comment dois-je réparer cette méthode afin qu'elle adhère à "SRP"?

Answer 1

Je pense que la chose importante à garder à l'esprit est de garder vos responsabilités sur le même niveau d'abstraction. Cela signifie que si vous divisez cette fonction en plusieurs morceaux, oui, elle accomplira beaucoup de choses, cependant, elle aura une responsabilité - orchestrer le travail à un niveau d'abstraction plus bas.

À titre d'exemple est ici la première partie de votre fonction après quelques refactoring:

public List<ProjectElement> getFavoriteProjectElementsSortedDescendingByTotalWorkHoursInLast60Days(Employee owner) { 
    // All Favorite Project Elements of Owner 
    final List<ProjectElement> favoriteProjectElementsOfOwner 
      = (List<ProjectElement>) (List<?>) favoriteBusinessObjectHelper.getAllFavoriteObjectsForBusinessObjectType(BusinessObjectType.PROJECT_ELEMENT.toInt()); 

    // Activities by Favorite Project Elements 
    final List<Activity> favoriteProjectElementActivitiesForLast60Days 
      = activityFinder.findByInProjectElementsForLastGivenDates(owner, favoriteProjectElementsOfOwner, 60); 

    // Create a Map with Favorite Project Element - Calculated Work 
    final Map<ProjectElement, Long> projectElementTotalWorkMap = this.makeFacoriteProjectMap(favoriteProjectElementActivitiesForLast60Days) 

La fonctionnalité qui génère votre projectElementTotalWorkMap a été déplacé à une fonction privée. Donc maintenant ce n'est plus à vos fonctions principales de comprendre comment construire la carte.S'il y a une erreur dans la façon dont la carte est générée, il vous suffit de regarder dans la fonction que vous le déplacez:

private Map<ProjectElement, Long> projectElementTotalWorkMap makeFacoriteProjectMap(List<Activity> favoriteProjectElementActivitiesForLast60Days) 
{ 
    Map<ProjectElement, Long> projectElementTotalWorkMap= new HashMap<ProjectElement, Long>(); 
    for (Activity activity : favoriteProjectElementActivitiesForLast60Days) { 
     final ProjectElement activityProjectElement = activity.getProjectElement(); 
     if (!projectElementTotalWorkMap.containsKey(activityProjectElement)) { 
      projectElementTotalWorkMap.put(activityProjectElement, 0L); 
     } 
     final long calculatedWork = activity.getCalculatedWork(); 
     final long totalCalculatedWork = projectElementTotalWorkMap.get(activityProjectElement) + calculatedWork; 
     projectElementTotalWorkMap.put(activityProjectElement, totalCalculatedWork); 
    } 

    return projectElementTotalWorkMap; 
} 

Auparavant, si vous avez découvert une erreur dans la façon dont vous générer cette carte, vous aurait dû changer la fonction principale. La fonction principale devrait également changer si, par exemple, les choses se passent dans le mauvais ordre.

L'orchestration est désormais la principale fonction uniquement.

Ce même refactor peut être appliqué pour générer votre projectElementsSorted ArrayList, après quoi la fonction getFavoriteProjectElementsSortedDescendingByTotalWorkHoursInLast60Days aurait 1 responsabilité - orchestrant les étapes pour vous fournir la liste des heures de travail triées au cours des 60 derniers jours.

Il y aura d'autres petites fonctions privées ayant chacune leur propre responsabilité. Même avec autant de fonctions privées qui font chacune son affaire, la classe principale de ce code a toujours une responsabilité envers le monde extérieur.

Pour faire un lien avec votre exemple d'aboiement de chien, je ferai une autre analogie. Disons que vous avez un entraîneur pour entraîner votre chien à effectuer une routine dans le cirque. A la fin de la journée, le chien doit être en mesure de le faire:

class Dog 
{ 
    public void PerformRoutine() 
    { 
     this.bark(); 
     this.sit(); 
     this.walkInCircle(); 
     this.rollOver(); 
    } 
    //... 
} 

Même si cette fonction fait beaucoup de choses, la seule raison de cette fonction pour changer jamais serait si les changements de routine. Cependant, si le bruit que fait le chien quand il aboie doit changer, vous ne regarderez pas ici. Tout dans la fonction PerformRoutine est sur le même niveau d'abstraction.

Dans le contexte des applications d'entreprise, vous avez souvent une couche service/api qui orchestre l'enregistrement, le chargement et l'appel des objets de domaine. Ces fonctions semblent violer SR mais elles font partie de la même responsabilité. L'enregistrement et le chargement sont sous la responsabilité de vos dépôts. La logique actuelle est la responsabilité de vos objets de domaine. En fait, ces API/services ne font qu'une chose: orchestrer et déléguer à d'autres services.

J'espère que cela peut vous aider.

EDIT: Pour répondre à votre question sur ce que la valeur de tout cela est:

Prenons l'exemple de chien précédent sans SR:

class Dog 
{ 
    public IEnumerator PerformRoutine() 
    { 
     this.audioController.PlaySound("c:/bar.mp3"); 
     this.animationControl.Play("Sit.anim"); 

     yield return new WaitForSeconds(2); 

     this.animationController.Play("stand.anim");   

     yield return new WaitForSeconds(1); 

     foreach(Vector3 pos in this.WalkPositions) 
     { 
      this.animationController.Play("walk.anim"); 
      this.position.lerpTo(pos, 5.0f); // move to position over 5 seconds. 

     } 

     this.animationController.Play("roll.anim"); 
    } 
    //... 
} 

Tout nouveau membre de l'équipe doit maintenant comprendre exactement la détails de mise en œuvre de cette fonction afin de comprendre ce qu'il fait vraiment. Ceci est déjà fortement simplifié.

Si vous demandez à un nouveau membre de l'équipe (ou même si vous revisitez votre ancien code), vous voudriez idéalement jeter un coup d'œil dessus et comprendre ce qu'il fait. Le nom de la fonction "PerformRoutine" ne prépare pas le lecteur pour l'animation ou la logique de mouvement.

Si vous êtes renvoyé à ce code parce que le chien marche dans un carré au lieu d'un cercle, il est plus difficile d'ajuster ce code.

Appliquez maintenant ce même raisonnement dans une solution d'entreprise avec des centaines de milliers de lignes de code et des milliers de fonctions. Il devient si ingérable que finalement vous voulez quitter.

Voici un autre exemple d'intégration de REAL LIFE que j'ai fait pour une entreprise d'investissement. Voici à peu près le code 3ème partie qui a pris quelques jours de congé de mon calendrier de développement:

public List<Event> getEventData() 
{ 
    var toReturn = new List<Event>(this.eventData); 
    this.eventData.Clear(); 
    return toReturn; 
} 

Voici. Appeler cette fonction deux fois de suite donne des sorties radicalement différentes parce que l'auteur ne s'est pas soucié de SR ou de nommer correctement.

Avec un minimum d'effort, cela aurait pu être 2 fonctions - getEventDate() et clearEventData(). J'aurais même réglé pour 1 fonction avec un nom différent - getAndClearEventData() bien que ce dernier viole SR.