Le principe de responsabilité unique s'applique-t-il aux fonctions?

17

Selon Robert C. Martin, le SRP déclare que:

Il ne devrait jamais y avoir plus d'une raison pour qu'une classe change.

Cependant, dans son livre Clean Code , chapitre 3: Fonctions, il montre le bloc de code suivant:

    public Money calculatePay(Employee e) throws InvalidEmployeeType {
        switch (e.type) {
            case COMMISSIONED:
                return calculateCommissionedPay(e);
            case HOURLY:
                return calculateHourlyPay(e);
            case SALARIED:
                return calculateSalariedPay(e);
            default:
                throw new InvalidEmployeeType(e.type);
        }
    }

Et puis déclare:

Il y a plusieurs problèmes avec cette fonction. Tout d'abord, il est important et lorsque de nouveaux types d'employés sont ajoutés, il augmentera. Deuxièmement, il fait très clairement plus d'une chose. Troisièmement, il viole le principe de responsabilité unique (PRS) car il y a plus d'une raison pour qu'il change . [c'est moi qui souligne]

Tout d'abord, je pensais que le SRP était défini pour les classes, mais il s'avère qu'il est également applicable aux fonctions. Deuxièmement, comment cette fonction a-t-elle plus d'une raison de changer ? Je ne peux que le voir changer en raison d'un changement d'employé.

Enrique
la source
5
Cela ressemble à un cas d'école pour le polymorphisme.
wchargin
C'est un sujet très intéressant. Y a-t-il une chance que vous ajoutiez la solution suivante à ce problème? Je dirais que l'on a mis une fonction de calcul de rémunération dans chaque classe d'employés, mais ce serait une mauvaise affaire car chaque classe d'employés peut être modifiée en raison de: 1. Les calculs de paiement. 2. ajouter plus de propriétés à la classe etc.
Stav Alfi

Réponses:

13

Un détail souvent oublié du principe de responsabilité unique est que les «raisons du changement» sont regroupées par acteurs du cas d'utilisation (vous pouvez voir une explication complète ici ).

Ainsi, dans votre exemple, la calculatePayméthode devra être modifiée chaque fois que de nouveaux types d'employés seront nécessaires. Étant donné qu'un type d'employé peut n'avoir rien à voir avec un autre, ce serait une violation du principe si vous les maintenez ensemble, car le changement affecterait différents groupes d'utilisateurs (ou acteurs de cas d'utilisation) dans le système.

Maintenant, pour savoir si le principe s'applique aux fonctions: même si vous avez une violation dans une seule méthode, vous modifiez toujours une classe pour plusieurs raisons, c'est donc toujours une violation de SRP.

MichelHenrich
la source
1
J'ai essayé de regarder la vidéo YouTube liée, mais après 10 minutes dedans sans mentionner le regroupement par des acteurs de cas d'utilisation, j'ai abandonné. Les 6 premières minutes parlent toutes d'entropie, sans raison apparente. Si vous avez donné un emplacement dans la vidéo où il commence à en discuter, ce serait utile.
Michael Shaw
@MichaelShaw Essayez de regarder à partir de 10h40. L'oncle Bob mentionne que le code "changera pour différentes raisons, à cause de personnes différentes". Je pense que c'est peut-être ce que MichelHenrich essaie de nous montrer.
Enrique
Vous avez terminé de regarder l'intégralité de la vidéo YouTube de 50 minutes, dont la majorité ne concernait pas ce qu'elle devait clarifier. J'ai remarqué à 16h00 qu'il était déjà passé de ce sujet, et il n'y est jamais revenu. L '"explication" se résume essentiellement à ceci: "la responsabilité" dans SRP ne signifie pas que, cela signifie "différentes raisons de changement", ce qui signifie vraiment "des changements à la demande de différentes personnes", ce qui signifie vraiment "des changements à la demande de différents rôles que les gens jouent ". L '«explication» ne clarifie rien, elle remplace un mot vague par un autre.
Michael Shaw
2
@MichaelShaw comme dans la citation du livre, si vous devez introduire différents types d'employés, vous devez changer la classe Employé. Différents rôles peuvent être responsables du paiement de différents types d'employés, donc dans ce cas, la classe Employé doit être modifiée pour plus d'un rôle, d'où la violation de SRP.
MichelHenrich
1
@MichaelShaw oui, vous avez raison - SRP dépend de la façon dont l'organisation est organisée. C'est exactement pourquoi j'ajoute "peut" ou "pourrait" à tous mes commentaires :). Cependant, même dans ces cas, bien que le code ne puisse pas violer SRP, il viole sûrement OCP.
MichelHenrich
3

À la page 176, Chapitre 12: Emergence, dans la section intitulée Classes et méthodes minimales, le livre apporte une correction, en déclarant:

Afin de réduire la taille de nos classes et méthodes, nous pourrions créer trop de petites classes et méthodes. Donc, cette règle suggère que nous gardons également notre nombre de fonctions et de classes bas

et

Le nombre élevé de classes et de méthodes est parfois le résultat d'un dogmatisme inutile.

Évidemment, il parle de dogmatisme en suivant le SRP pour décomposer parfaitement les petites méthodes innocentes comme calculatePay()ci-dessus.

Mike Nakis
la source
3

Lorsque M. Martin applique le PÉR à une fonction, il étend implicitement sa définition du PÉR. Comme le SRP est une formulation spécifique à OO d'un principe général, et comme c'est une bonne idée lorsqu'il est appliqué à des fonctions, je ne vois pas de problème avec cela (même si cela aurait été bien s'il l'avait explicitement inclus dans le définition).

Je ne vois pas plus d'une raison de changer non plus, et je ne pense pas que penser au PÉR en termes de «responsabilités» ou de «raisons de changer» soit utile. Essentiellement, l'objectif du SRP est que les entités logicielles (fonctions, classes, etc.) doivent faire une chose et bien le faire.

Si vous jetez un œil à ma définition, elle n'est pas moins vague que la formulation habituelle du PÉR. Le problème avec les définitions habituelles du PÉR n'est pas qu'elles sont trop vagues, mais qu'elles essaient d'être trop précises sur quelque chose qui est essentiellement vague.

Si vous regardez ce qui se calculatePaypasse, cela fait clairement une seule chose: l'envoi basé sur le type. Étant donné que Java propose des méthodes intégrées de répartition basée sur le type, il calculatePayest inélégant et non idiomatique, il doit donc être réécrit, mais pas pour les raisons indiquées.

Michael Shaw
la source
-2

Vous avez raison @Enrique. Peu importe qu'il s'agisse d'une fonction ou d'une méthode d'une classe, le SRP signifie que dans ce bloc de code, vous ne faites qu'une seule chose.

La déclaration «raison de changer» est parfois un peu trompeuse, mais si vous changez calculateSalariedPayou calculateHourlyPayou l'énumération de Employee.typevous devez changer cette méthode.

Dans votre exemple, la fonction:

  • vérifie le type de l'employé
  • appelle une autre fonction qui calcule l'argent en fonction du type

À mon avis, ce n'est pas directement une violation de la SRP, car les commutateurs et les appels ne peuvent pas être écrits plus courts, si vous pensez à l'employé et que les méthodes existent déjà. Quoi qu'il en soit, il s'agit d'une violation claire du principe ouvert-fermé (OCP) car vous devez ajouter des déclarations de cas si vous ajoutez des types d'employés, c'est donc une mauvaise mise en œuvre: refactorisez-la.

Nous ne savons pas comment le « argent » doit être calculée, mais la meilleure façon est d'avoir Employeecomme interface et certaines implémentations concrètes avec des getMoneyméthodes. Dans ce cas, toute la fonction est inutile.

S'il est plus compliqué de le calculer, on pourrait utiliser le modèle de visiteur qui n'est pas non plus 100% SRP mais c'est plus OCP qu'un boîtier de commutation.

Aitch
la source
2
Je ne sais pas comment vous pouvez lister 2 choses que fait la fonction, mais dites que ce n'est pas une violation SRP.
JeffO
@JeffO: Ce n'est pas 2 choses, c'est 2 parties d'une chose: appeler la fonction appropriée en fonction du type.
Michael Shaw