L'ajout d'un type de retour à une méthode de mise à jour enfreint-il le «principe de responsabilité unique»?

J'ai une méthode qui met à jour les données des employés dans la base de données. La Employeeclasse est immuable, donc "mettre à jour" l'objet signifie réellement instancier un nouvel objet.

Je souhaite que la Updateméthode retourne une nouvelle instance Employeeavec les données mises à jour, mais puisque je peux maintenant dire que la méthode consiste à mettre à jour les données des employés et à extraire de la base de données un nouvel Employeeobjet . Cela enfreint-il le principe de responsabilité unique ?

L'enregistrement de base de données lui-même est mis à jour. Ensuite, un nouvel objet est instancié pour représenter cet enregistrement.

Comme pour toute règle, je pense que l’important ici est de considérer le but de la règle, son esprit, et de ne pas s’embourber à analyser exactement comment la règle a été formulée dans un manuel et comment l’appliquer à ce cas. Nous n'avons pas besoin d'aborder cela comme des avocats. Le but du règlement est de nous aider à écrire de meilleurs programmes. Ce n’est pas comme si l’écriture de programmes avait pour but de faire respecter les règles.

La règle de la responsabilité unique a pour but de rendre les programmes plus faciles à comprendre et à gérer en permettant à chaque fonction d’agir de manière cohérente et autonome.

Par exemple, j’ai écrit une fois une fonction que j’ai appelée quelque chose comme "checkOrderStatus", qui déterminait si une commande était en attente, expédiée, en rupture de stock, etc., et renvoyait un code indiquant laquelle. Ensuite, un autre programmeur est arrivé et a modifié cette fonction pour mettre à jour la quantité en stock lors de l’expédition de la commande. Cela violait gravement le principe de responsabilité unique. Un autre programmeur lisant ce code plus tard verrait le nom de la fonction, verrait comment la valeur de retour était utilisée et pourrait ne jamais soupçonner qu’il avait mis à jour la base de données. Quelqu'un qui aurait besoin d'obtenir le statut de la commande sans mettre à jour la quantité en stock se trouverait dans une position inconfortable: devrait-il écrire une nouvelle fonction qui duplique la partie du statut de la commande? Ajouter un drapeau pour lui dire si faire la mise à jour de la base de données? Etc.

Par contre, je ne voudrais pas chercher ce qui constitue "deux choses". Récemment, j'ai écrit une fonction qui envoie les informations client de notre système vers le système de notre client. Cette fonction effectue un certain reformatage des données pour répondre à leurs besoins. Par exemple, nous avons des champs qui peuvent être nuls sur notre base de données, mais ils n'autorisent pas les nuls, nous devons donc remplir un texte factice, "non spécifié" ou j'oublie les mots exacts. On peut soutenir que cette fonction fait deux choses: reformater les données ET les envoyer. Mais j’ai très délibérément mis cela dans une seule fonction plutôt que d’avoir "reformater" et "envoyer" parce que je ne veux jamais, jamais envoyer sans reformater. Je ne veux pas que quelqu'un écrive un nouvel appel sans se rendre compte qu'il doit appeler reformater puis envoyer.

Dans votre cas, mettre à jour la base de données et renvoyer une image de la notice écrite semble être deux choses qui pourraient bien aller ensemble de manière logique et inévitable. Je ne connais pas les détails de votre candidature, je ne peux donc pas dire avec certitude si c'est une bonne idée ou non, mais cela semble plausible.

Si vous créez un objet en mémoire qui contient toutes les données de l'enregistrement, effectuez les appels de base de données pour l'écrire, puis renvoyez l'objet, cela a beaucoup de sens. Vous avez l'objet entre vos mains. Pourquoi ne pas simplement le rendre? Si vous n'avez pas renvoyé l'objet, comment l'appelant l'obtiendrait-il? Devrait-il lire la base de données pour obtenir l'objet que vous venez d'écrire? Cela semble plutôt inefficace. Comment trouverait-il le disque? Connaissez-vous la clé primaire? Si quelqu'un déclare qu'il est "légal" pour la fonction d'écriture de renvoyer la clé primaire afin que vous puissiez relire l'enregistrement, pourquoi ne pas simplement renvoyer l'intégralité de l'enregistrement afin que vous n'ayez pas à le faire? Quelle est la différence?

D'autre part, si la création de l'objet est un travail assez différent de l'écriture de l'enregistrement de base de données, et qu'un appelant peut très bien vouloir effectuer l'écriture mais ne pas créer l'objet, cela peut être une perte de temps. Si un appelant peut vouloir l'objet mais ne pas écrire, vous devez alors fournir un autre moyen de l'obtenir, ce qui peut signifier l'écriture de code redondant.

Mais je pense que le scénario 1 est plus probable, donc je dirais, probablement pas de problème.

Le concept du SRP est d'empêcher les modules de faire deux choses différentes quand ils sont réalisés individuellement, ce qui permet un meilleur entretien et moins de spaghettis dans le temps. Comme le dit le SRP, "une raison de changer".

Dans votre cas, si vous aviez une routine qui "met à jour et retourne l'objet mis à jour", vous ne modifiez toujours l'objet qu'une fois, ce qui lui donne un motif de modification. Que vous retourniez l'objet immédiatement n'est ni ici ni là-bas, vous opérez toujours sur cet objet unique. Le code n’est responsable que d’une seule chose.

Le SRP ne consiste pas vraiment à essayer de réduire toutes les opérations à un seul appel, mais à réduire les opérations sur lesquelles vous opérez et la façon dont vous les exploitez. Donc, une seule mise à jour qui renvoie l'objet mis à jour convient.

Comme toujours, c'est une question de degré. Le SRP devrait vous empêcher d'écrire une méthode qui extrait un enregistrement d'une base de données externe, effectue une transformation de Fourier rapide dessus et met à jour un registre de statistiques global avec le résultat. Je pense que presque tout le monde serait d'accord pour dire que ces choses devraient être faites par différentes méthodes. Postuler une responsabilité unique pour chaque méthode est tout simplement le moyen le plus économique et le plus mémorable de faire valoir cet argument.

À l’autre extrémité du spectre se trouvent des méthodes qui fournissent des informations sur l’état d’un objet. Le typique isActivea donné cette information sous sa responsabilité unique. Tout le monde est probablement d'accord pour dire que ça va.

Certains ont jusqu'à présent étendu le principe selon lequel ils envisagent de renvoyer un succès à une autre responsabilité que celle d'effectuer l'action dont le succès a été rapporté. Sous une interprétation extrêmement stricte, cela est vrai, mais puisque l’alternative serait d’appeler une deuxième méthode pour obtenir le statut de succès, ce qui complique la tâche de l’appelant, de nombreux programmeurs acceptent parfaitement de renvoyer les codes de succès d’une méthode avec des effets secondaires.

Retourner le nouvel objet est un pas de plus sur la route vers de multiples responsabilités. Demander à l'appelant de faire un deuxième appel pour un objet entier est légèrement plus raisonnable que d'exiger un deuxième appel simplement pour voir si le premier a réussi ou non. Néanmoins, de nombreux programmeurs envisageraient de renvoyer parfaitement le résultat d’une mise à jour. Bien que cela puisse être interprété comme deux responsabilités légèrement différentes, ce n’est certainement pas l’un des abus flagrants qui ont inspiré le principe.

Cela enfreint-il le principe de responsabilité unique?

Pas nécessairement. Si quelque chose, cela viole le principe de la séparation commande-requête .

La responsabilité est

mettre à jour les données des employés

avec la compréhension implicite que cette responsabilité inclut le statut de l'opération; par exemple, si l'opération échoue, une exception est levée, si l'opération réussit, l'employé de mise à jour est renvoyé, etc.

Encore une fois, tout est une question de degré et de jugement subjectif.

Alors, qu'en est-il de la séparation commande-requête?

Eh bien, ce principe existe, mais il est très courant de renvoyer le résultat d’une mise à jour.

(1) Java Set#add(E)ajoute l'élément et retourne son inclusion précédente dans l'ensemble.

if (visited.add(nodeToVisit)) {
    // perform operation once
}

Ceci est plus efficace que l'alternative CQS, qui peut avoir à effectuer deux recherches.

if (!visited.contains(nodeToVisit)) {
    visited.add(nodeToVisit);
    // perform operation once
}

(2) Les primitives courantes permettant la programmation simultanée sont les suivantes: comparer et permuter , extraire et ajouter , et tester et définir . Ces modèles apparaissent souvent, des instructions de processeur de bas niveau aux collections simultanées de haut niveau.

L'unique responsabilité concerne une classe qui n'a pas besoin de changer pour plus d'une raison.

Par exemple, un employé possède une liste de numéros de téléphone. Lorsque vous gérez les numéros de téléphone (vous pouvez éventuellement ajouter des indicatifs de pays), cela ne devrait pas changer la classe d'employés.

Je n'aurais pas besoin que la classe d'employés ait besoin de savoir comment elle s'enregistre dans la base de données, car cela changerait pour tenir compte des changements d'employés et de la manière dont les données sont stockées.

De même, il ne devrait pas y avoir de méthode CalculateSalary dans la classe employee. Une propriété de salaire est acceptable, mais le calcul de l'impôt, etc. doit être effectué ailleurs.

Mais que la méthode Update retourne ce qu'elle vient de mettre à jour convient.

Wrt. le cas spécifique:

Employee Update(Employee, name, password, etc) (En fait, en utilisant un générateur car j'ai beaucoup de paramètres).

Il semble que la Updateméthode prenne un Employeeparamètre existant en premier pour identifier (?) Le salarié existant et un ensemble de paramètres à modifier pour ce salarié.

Je ne pense que cela pourrait être fait plus propre. Je vois deux cas:

(a) Employee contient effectivement une base de données / un identifiant unique grâce auquel il peut toujours être identifié dans la base de données. (Autrement dit, vous n'avez pas besoin du jeu de valeurs d'enregistrement complet pour le trouver dans la base de données.

Dans ce cas, je préférerais une void Update(Employee obj)méthode, qui trouve simplement l'enregistrement existant par ID, puis met à jour les champs à partir de l'objet transmis. Ou peut-être unvoid Update(ID, EmployeeBuilder values)

Une variante de ce que j'ai trouvé utile est d'avoir uniquement une void Put(Employee obj)méthode qui insère ou met à jour, selon que l'enregistrement (par ID) existe.

(b) la fiche complète existante est nécessaire pour DB recherche, dans ce cas , il pourrait encore plus logique d'avoir: void Update(Employee existing, Employee newData).

Autant que je sache, je dirais bien que la responsabilité de construire un nouvel objet (ou des valeurs de sous-objets) à stocker et de le stocker réellement est orthogonale, donc je les séparerais.

Les exigences simultanées mentionnées dans d'autres réponses (lecture / extraction / comparaison-échange atomiques, etc.) ne sont pas un problème dans le code de base de données sur lequel j'ai travaillé jusqu'à présent. Lorsque vous parlez à une base de données, je pense que cela devrait être traité normalement au niveau de la transaction et non au niveau de l'instruction individuelle. (Cela ne veut pas dire qu'il pourrait ne pas y avoir de conception dans laquelle "atomique" Employee[existing data] Update(ID, Employee newData)n'aurait aucun sens, mais avec l'accès à la base de données, ce n'est pas quelque chose que je vois normalement.)

Jusqu'à présent, tout le monde ici parle de cours. Mais réfléchissez-y du point de vue de l'interface.

Si une méthode d'interface déclare un type de retour et / ou une promesse implicite concernant la valeur de retour, chaque implémentation doit alors renvoyer l'objet mis à jour.

Donc, vous pouvez demander:

• Pouvez-vous penser aux implémentations possibles qui ne veulent pas retourner un nouvel objet?
• Pouvez-vous penser aux composants qui dépendent de l'interface (en ayant une instance injectée), qui n'ont pas besoin de l'employé mis à jour en tant que valeur de retour?

Pensez également aux objets fictifs pour les tests unitaires. Évidemment, il sera plus facile de se moquer sans la valeur de retour. Et les composants dépendants sont plus faciles à tester si leurs dépendances injectées ont des interfaces plus simples.

Sur la base de ces considérations, vous pouvez prendre une décision.

Et si vous le regrettez plus tard, vous pouvez toujours introduire une deuxième interface, avec des adaptateurs entre les deux. Bien sûr, de telles interfaces supplémentaires augmentent la complexité de la composition, c'est donc un compromis.

Votre approche est bien. L'immuabilité est une affirmation forte. La seule chose que je voudrais demander est la suivante: existe-t-il un autre endroit où vous construisez l'objet? Si votre objet n'est pas immuable, vous devrez répondre à des questions supplémentaires car "State" est introduit. Et le changement d'état d'un objet peut se produire pour différentes raisons. Ensuite, vous devez connaître vos cas et ils ne doivent pas être redondants ou divisés.