Comment justifiez-vous l'écriture de davantage de code en suivant les pratiques de code propre?

106

Note du modérateur Dix- sept réponses
ont déjà été posées sur cette question. Avant de poster une nouvelle réponse, veuillez lire les réponses existantes et assurez-vous que votre point de vue n'est pas déjà suffisamment couvert.

J'ai suivi certaines des pratiques recommandées dans le livre "Code propre" de Robert Martin, en particulier celles qui s'appliquent au type de logiciel avec lequel je travaille et celles qui me semblent logiques (je ne le suis pas comme un dogme) .

Un effet secondaire que j’ai remarqué, cependant, est que le code «propre» que j’écris est plus codé que si je ne suivais pas certaines pratiques. Les pratiques spécifiques qui mènent à ceci sont:

  • Conditionnel d'encapsulation

Donc au lieu de

if(contact.email != null && contact.emails.contains('@')

Je pourrais écrire une petite méthode comme celle-ci

private Boolean isEmailValid(String email){...}
  • Remplacement d'un commentaire en ligne par une autre méthode privée, afin que le nom de la méthode se décrive lui-même plutôt que d'avoir un commentaire en ligne par dessus
  • Une classe ne devrait avoir qu'une seule raison de changer

Et quelques autres. Le fait est que ce qui pourrait être une méthode de 30 lignes finit par être une classe, à cause des méthodes minuscules qui remplacent les commentaires et encapsulent les conditionnelles, etc. Lorsque vous réalisez que vous avez autant de méthodes, alors il mettez toutes les fonctionnalités dans une seule classe, alors que cela aurait dû être une méthode.

Je suis conscient que toute pratique poussée à l'extrême peut être nocive.

La question concrète à laquelle je cherche une réponse est:

Est-ce un sous-produit acceptable d'écrire un code propre? Si tel est le cas, quels arguments puis-je utiliser pour justifier le fait que davantage de lettres de crédit ont été écrites?

L'organisation ne se préoccupe pas spécifiquement de plus de LOC, mais plus de LOC peut donner lieu à de très grandes classes (qui pourraient être remplacées par une méthode longue sans un tas de fonctions utilitaires utilisables une seule fois pour des raisons de lisibilité).

Lorsque vous voyez une classe assez grande, cela donne l’impression que la classe est suffisamment occupée et que sa responsabilité est terminée. Vous pourriez donc finir par créer plus de classes pour réaliser d'autres fonctionnalités. Le résultat est alors beaucoup de classes, toutes faisant "une chose" à l'aide de nombreuses méthodes de petits assistants.

CECI est la préoccupation spécifique ... ces classes pourraient être une classe unique qui réalise toujours "une chose", sans l'aide de nombreuses petites méthodes. Ce pourrait être une classe unique avec peut-être 3 ou 4 méthodes et quelques commentaires.

CommonCoreTawan
la source
98
Si votre organisation utilise uniquement le LOC en tant que métrique pour vos bases de code, justifier par un code vierge est sans espoir.
Kilian Foth
24
Si votre objectif est la facilité de maintenance, la LDC n'est pas la meilleure mesure à prendre - c'est l'un d'entre eux, mais il y a beaucoup plus à considérer que de rester bref.
Zibbobz
29
N’est pas une réponse, mais une remarque à faire: il existe toute une sous-communauté pour écrire du code avec aussi peu de lignes / symboles que possible. codegolf.stackexchange.com On peut affirmer que la plupart des réponses ne sont pas aussi lisibles qu'elles pourraient l'être.
Antitheos
14
Apprenez les raisons qui sous-tendent toutes les meilleures pratiques et pas seulement les règles Suivre des règles sans raison est un culte du cargo. Chaque règle a sa propre raison.
Gherman
9
Juste en passant, et en vous servant de votre exemple, vous pouvez parfois pousser les choses à des méthodes pour vous faire penser: "Peut-être qu'une fonction de bibliothèque peut-elle le faire". Par exemple, pour valider une adresse électronique, vous pouvez créer une adresse System.Net.Mail.MailAddress qui la validera pour vous. Vous pouvez ensuite (espérons-le) faire confiance à l'auteur de cette bibliothèque pour bien faire les choses. Cela signifie que votre base de code aura plus d'abstractions et sa taille diminuera.
Gregory Currie

Réponses:

130

... nous formons une très petite équipe prenant en charge une base de code relativement importante et non documentée (dont nous avons hérité), de sorte que certains développeurs / gestionnaires voient l'intérêt d'écrire moins de code pour que les choses soient faites, de sorte que nous avons moins de code à maintenir.

Ces personnes ont correctement identifié quelque chose: elles veulent que le code soit plus facile à gérer. Là où ils se sont trompés, c'est en supposant que moins il y a de code, plus il est facile à maintenir.

Pour que le code soit facile à gérer, il doit être facile à modifier. De loin, le moyen le plus simple d’obtenir un code facile à modifier est d’avoir un ensemble complet de tests automatisés qui échoueront si votre modification est révolutionnaire. Les tests sont du code, alors écrire ces tests va grossir votre base de code. Et c'est une bonne chose.

Deuxièmement, pour déterminer ce qui doit changer, votre code doit être à la fois facile à lire et à raisonner. Un code très concis, dont la taille est réduite pour garder le compte à rebours est très improbable. De toute évidence, il faut trouver un compromis car un code plus long prendra plus de temps à lire. Mais si c'est plus rapide à comprendre, alors ça vaut le coup. S'il n'offre pas cet avantage, cette verbosité cesse d'être un avantage. Mais si un code plus long améliore la lisibilité, c'est encore une bonne chose.

David Arno
la source
27
"De loin, le moyen le plus simple d'obtenir un code facile à modifier consiste à disposer d'un ensemble complet de tests automatisés qui échoueront si votre modification est décisive." Ce n'est tout simplement pas vrai. Les tests requièrent un travail supplémentaire pour chaque changement de comportement, car ils doivent également être modifiés , et ceci est inhérent à la conception. Beaucoup affirment même que cela rend le changement plus sûr, mais cela rend également les changements plus difficiles.
Jack Aidley
63
Bien sûr, mais le temps perdu à maintenir ces tests est réduit au minimum au moment où vous auriez perdu le diagnostic et la correction des bogues évités par les tests.
MetaFight
29
@JackAidley, le fait de devoir modifier les tests avec le code peut donner l’apparence de travail supplémentaire, mais seulement si l’on ignore les bogues difficiles à trouver que le changement de code non testé introduira et qui ne seront souvent retrouvés qu’après leur expédition. . Ce dernier offre simplement l'illusion de moins de travail.
David Arno
31
@ JackAidley, je suis complètement en désaccord avec vous. Les tests facilitent le changement de code. Je conviens que ce code mal conçu, qui est trop étroitement couplé et donc étroitement lié aux tests, peut être difficile à modifier, mais un code bien structuré et bien testé est simple à modifier dans mon expérience.
David Arno
22
@JackAidley Vous pouvez refactoriser beaucoup sans changer aucune API ou interface. Cela signifie que vous pouvez devenir fou en modifiant le code sans avoir à changer une seule ligne dans les tests unitaires ou fonctionnels. Autrement dit, si vos tests ne testent pas une implémentation spécifique.
Eric Duminil
155

Oui, c'est un sous-produit acceptable, et la justification est qu'il est maintenant structuré de sorte que vous n'avez pas à lire la plupart du code la plupart du temps. Au lieu de lire une fonction de 30 lignes chaque fois que vous effectuez un changement, vous lisez une fonction de 5 lignes pour obtenir le flux global, et peut-être quelques fonctions auxiliaires si votre modification touche cette zone. Si votre nouvelle classe "extra" est appelée EmailValidatoret que vous savez que votre problème n’est pas lié à la validation du courrier électronique, vous pouvez ignorer la lecture.

Il est également plus facile de réutiliser des éléments plus petits, ce qui a tendance à réduire le nombre de lignes pour l'ensemble de votre programme. Un EmailValidatorpeut être utilisé partout. Certaines lignes de code validant le courrier électronique mais regroupées avec le code d'accès à la base de données ne peuvent pas être réutilisées.

Et ensuite, réfléchissez à ce qui doit être fait si les règles de validation des courriers électroniques doivent être modifiées - vous préférez: un emplacement connu; ou de nombreux endroits, peut-être en manque quelques-uns?

Karl Bielefeldt
la source
10
La réponse est bien meilleure alors que «les tests unitaires résolvent tous vos problèmes»
Dirk Boer
13
Cette réponse touche un point clé qui manque toujours à Oncle Bob et à ses amis - le refactoring en petites méthodes n'aide que si vous n'avez pas à lire toutes les petites méthodes pour comprendre ce que fait votre code. Il est judicieux de créer une classe distincte pour valider les adresses électroniques. Tirer le code iterations < _maxIterationsdans une méthode appelée ShouldContinueToIterateest stupide .
BJ Myers
4
@DavidArno: "être utile"! = "Résout tous vos problèmes"
Christian Hackl
2
@DavidArno: Quand on se plaint de personnes qui impliquent que les tests unitaires "résolvent tous vos problèmes", elles désignent évidemment les personnes qui impliquent que les tests unitaires résolvent, ou du moins contribuent à la résolution de presque tous les problèmes de génie logiciel. Je pense que personne n'accuse personne de suggérer le test unitaire comme moyen de mettre fin à la guerre, à la pauvreté et à la maladie. Une autre façon de le dire est que la surévaluation extrême des tests unitaires dans de nombreuses réponses, non seulement à cette question, mais à SE en général, fait l'objet de critiques (à juste titre).
Christian Hackl
2
Bonjour @DavidArno, mon commentaire était clairement une hyperbole, pas un homme de paille;) Pour moi, c'est comme ça: je demande comment réparer ma voiture et faire venir des personnes religieuses qui me disent que je devrais vivre une vie moins pécheuse. En théorie, quelque chose mérite d’être discuté, mais cela ne m’aide pas vraiment à améliorer la réparation des voitures.
Dirk Boer
34

Bill Gates était réputé pour avoir déclaré: "Mesurer les progrès de la programmation à l'aide de lignes de code revient à mesurer les progrès de la construction de l'avion en termes de poids."

Je suis humblement d'accord avec ce sentiment. Cela ne veut pas dire qu'un programme doit s'efforcer d'avoir plus ou moins de lignes de code, mais que ce n'est pas ce qui compte en définitive pour créer un programme fonctionnel et fonctionnel. Il est utile de se rappeler que la raison ultime de l'ajout de lignes de code supplémentaires est qu'il est théoriquement plus lisible de cette façon.

On peut avoir des désaccords quant à savoir si un changement donné est plus ou moins lisible, mais je ne pense pas que vous auriez tort de modifier votre programme, car vous pensez que vous le rendez ainsi plus lisible. Par exemple, faire un isEmailValidpourrait être considéré comme superflu et inutile, surtout s'il est appelé une seule fois par la classe qui le définit. Cependant, je préférerais de beaucoup voir une isEmailValidcondition plutôt qu'une chaîne de conditions AND, dans laquelle je dois déterminer ce que chaque condition vérifie et pourquoi elle est vérifiée.

Vous rencontrez des problèmes lorsque vous créez une isEmailValidméthode qui a des effets secondaires ou qui vérifie des éléments autres que le courrier électronique, car cela est pire que de simplement tout écrire. C'est pire parce que c'est trompeur et que je risque de rater un bogue à cause de cela.

Bien que, de toute évidence, vous ne le fassiez pas dans ce cas, je vous encourage donc à continuer comme vous le faites. Vous devriez toujours vous demander si, en apportant le changement, il est plus facile à lire et si tel est votre cas, faites-le!

Neil
la source
1
Le poids de l'aéronef est une mesure importante, cependant. Et pendant la conception, le poids attendu est surveillé de près. Pas comme un signe de progrès, mais comme une contrainte. Les lignes de code de surveillance suggèrent que plus c'est mieux, alors que dans la conception des avions, moins de poids, c'est mieux. Donc, je pense que monsieur Gates aurait pu choisir une meilleure illustration pour son propos.
Jos
21
@jos sur l'équipe avec laquelle OP travaille, il semble que moins de lettres de crédit sont considérées comme "meilleures". Ce que Bill Gates disait, c'est que la LOC n'est pas liée au progrès de manière significative, tout comme dans le cas de la construction d'aéronefs, le poids n'est pas lié au progrès de manière significative. Un avion en construction peut avoir 95% de sa masse finale relativement rapidement, mais ce serait juste une coque vide sans système de contrôle, elle n’est pas achevée à 95%. Même chose dans le logiciel, si un programme a 100 000 lignes de code, cela ne signifie pas que 1 000 lignes fournissent 1% de la fonctionnalité.
M.Mindor
7
Le suivi des progrès est un travail difficile, n'est-ce pas? Pauvres gestionnaires.
jos
@ jos: dans le code aussi, il est préférable d'avoir moins de lignes pour la même fonctionnalité, si tout le reste est égal.
RemcoGerlich
@jos Lisez attentivement. Gates ne dit pas si le poids est une mesure importante pour un avion lui-même. Il dit que le poids est une mesure affreuse pour les progrès de la construction d’un avion. Après tout, par cette mesure, dès que vous avez toute la coque jetée au sol, vous avez pratiquement terminé votre travail, puisque cela équivaut vraisemblablement à 9x% du poids total de l'avion.
Voo
23

de sorte que certains développeurs / gestionnaires voient l'intérêt d'écrire moins de code pour faire avancer les choses, de sorte que nous ayons moins de code à maintenir

Il s’agit de perdre de vue le but réel.

Ce qui compte, c’est de réduire les heures consacrées au développement . Cela se mesure en temps (ou en un effort équivalent) et non en lignes de code.
Cela revient à dire que les constructeurs automobiles devraient construire leurs voitures avec moins de vis, car cela prend un temps non nul pour mettre chaque vis. Bien que cela soit correct du point de vue pédagogique, la valeur marchande d'une voiture n'est pas définie par son nombre de vis. ou n'a pas. Avant tout, une voiture doit être performante, sûre et facile à entretenir.

Le reste de la réponse montre comment un code propre peut générer des gains de temps.


Enregistrement

Prenez une application (A) qui n’a pas d’enregistrement. Maintenant, créez l'application B, qui est la même application A mais avec la journalisation. B aura toujours plus de lignes de code et vous devrez donc écrire plus de code.

Mais beaucoup de temps passera à enquêter sur les problèmes et les bugs et à déterminer ce qui ne va pas.

Pour l'application A, les développeurs seront bloqués en train de lire le code et devront reproduire en permanence le problème et parcourir le code pour trouver la source du problème. Cela signifie que le développeur doit tester du début à la fin, dans chaque couche utilisée, et doit observer chaque élément logique utilisé.
Peut-être a-t-il de la chance de le trouver immédiatement, mais peut-être que la réponse sera au dernier endroit qu'il pense regarder.

Pour l'application B, en supposant une journalisation parfaite, un développeur observe les journaux, peut identifier immédiatement le composant défectueux et sait désormais où chercher.

Cela peut être une question de minutes, heures ou jours économisés; en fonction de la taille et de la complexité de la base de code.


Régressions

Prenez l’application A, qui n’est pas du tout compatible avec le séchage.
Prenez l’application B, qui est DRY, mais qui a nécessité plus de lignes à cause des abstractions supplémentaires.

Une demande de modification est déposée, ce qui nécessite une modification de la logique.

Pour l'application B, le développeur modifie la logique (unique, partagée) en fonction de la demande de modification.

Pour l'application A, le développeur doit modifier toutes les instances de cette logique dans lesquelles il se souvient de son utilisation.

  • S'il parvient à se souvenir de toutes les instances, il devra toujours mettre en œuvre le même changement plusieurs fois.
  • S'il ne parvient pas à se souvenir de toutes les instances, vous avez maintenant affaire à une base de code incohérente qui se contredit. Si le développeur a oublié un élément de code rarement utilisé, il est possible que ce problème n'apparaisse plus avant pour les utilisateurs finaux. À ce moment-là, les utilisateurs finaux vont-ils identifier la source du problème? Même si c'est le cas, le développeur peut ne pas se souvenir de ce que le changement implique et devra trouver comment changer cette logique oubliée. Peut-être que le développeur ne travaille même plus dans l'entreprise à ce moment-là et que quelqu'un d'autre doit maintenant tout résoudre à partir de zéro.

Cela peut entraîner un énorme gaspillage de temps. Pas seulement en développement, mais à la chasse et à la découverte du bogue. L'application peut commencer à se comporter de manière erratique d'une manière que les développeurs ne peuvent pas comprendre facilement. Et cela conduira à de longues sessions de débogage.


Interchangeabilité avec les développeurs

Développeur A créé l’application A. Le code n’est ni propre, ni lisible, mais il fonctionne comme un charme et a été exécuté en production. Sans surprise, il n'y a pas de documentation non plus.

Le développeur A est absent pendant un mois en raison de vacances. Une demande de changement d'urgence est déposée. Il ne faut pas attendre encore trois semaines pour que Dev A revienne.

Le développeur B doit exécuter ce changement. Il doit maintenant lire l'intégralité de la base de code, comprendre comment tout fonctionne, pourquoi cela fonctionne et ce qu'il essaie d'accomplir. Cela prend des années, mais supposons qu'il puisse le faire en trois semaines.

Simultanément, l’application B (créée par dev B) a une urgence. Dev B est occupé, mais Dev C est disponible, même s'il ne connaît pas la base de code. Qu'est-ce qu'on fait?

  • Si nous continuons à travailler sur A et que C travaille à B, nous aurons deux développeurs qui ne savent pas ce qu'ils font et le travail est exécuté de manière sous-optimale.
  • Si nous retirons B de A et lui demandons de faire B, et que nous plaçons maintenant C sur A, alors tout le travail du développeur B (ou une partie importante de celui-ci) risque d'être rejeté. C'est potentiellement des jours / semaines d'effort gaspillé.

Dev A revient de ses vacances et constate que B n'a pas compris le code et l'a donc mal appliqué. Ce n'est pas la faute de B, puisqu'il a utilisé toutes les ressources disponibles, le code source n'était tout simplement pas lisible. Est-ce que A doit maintenant passer du temps à fixer la lisibilité du code?


Tous ces problèmes, et bien d’autres encore, finissent par perdre du temps . Oui, à court terme, le code propre nécessite plus d' efforts maintenant , mais il finira par payer des dividendes à l'avenir lorsque des bugs inévitables / changements doivent être pris en compte.

La direction doit comprendre qu'une tâche courte maintenant vous évitera plusieurs tâches longues dans le futur. Ne pas planifier est la planification à l'échec.

Si tel est le cas, quels arguments puis-je utiliser pour justifier le fait que davantage de lettres de crédit ont été écrites?

Mon explication au goto demande à la direction ce qu’elle préférerait: une application avec une base de code 100KLOC pouvant être développée en trois mois, ou une base de code 50KLOC pouvant être développée en six mois.

Ils choisiront évidemment le temps de développement le plus court, car la direction ne se soucie pas de KLOC . Les gestionnaires qui se concentrent sur KLOC font de la micro-gestion tout en ignorant ce qu’ils essaient de gérer.

Flater
la source
23

Je pense que vous devriez faire très attention à l’application de pratiques de «code propre» au cas où elles conduiraient à une plus grande complexité générale. La refactorisation prématurée est la racine de nombreuses mauvaises choses.

Extraire un conditionnel à une fonction conduit à un code plus simple à l'endroit où le conditionnel a été extrait , mais conduit à une complexité globale car vous disposez maintenant d'une fonction visible à partir de plusieurs points du programme. Vous ajoutez un léger fardeau de complexité à toutes les autres fonctions où cette nouvelle fonction est maintenant visible.

Je ne dis pas que vous ne devriez pas extraire le conditionnel, mais simplement que vous devriez considérer attentivement si vous en avez besoin.

  • Si vous souhaitez tester spécifiquement la logique de validation du courrier électronique. Ensuite, vous devez extraire cette logique dans une fonction distincte - probablement même la classe.
  • Si la même logique est utilisée à plusieurs endroits dans le code, vous devez évidemment l'extraire dans une seule fonction. Ne te répète pas!
  • Si la logique est évidemment une responsabilité distincte, par exemple, la validation du courrier électronique a lieu au milieu d’un algorithme de tri. La validation de l'e-mail changera indépendamment de l'algorithme de tri, ils doivent donc être dans des classes séparées.

Dans tout ce qui précède, il existe une raison pour l'extraction, au-delà du "code propre". En outre, vous ne douteriez probablement même pas que c'était la bonne chose à faire.

En cas de doute, je choisirais toujours le code le plus simple et le plus direct.

JacquesB
la source
7
Je dois admettre que transformer chaque condition en une méthode de validation peut introduire une complexité supplémentaire en matière de maintenance et de révision de code. Vous devez maintenant basculer dans le code pour vous assurer que vos méthodes conditionnelles sont correctes. Et qu'advient-il lorsque vous avez différentes conditions pour la même valeur? Vous pouvez maintenant avoir un cauchemar de nommage avec plusieurs petites méthodes qui ne sont appelées qu'une seule fois et qui se ressemblent pour la plupart.
pboss3010
7
Facilement la meilleure réponse ici. En particulier l'observation (dans le troisième paragraphe) que la complexité n'est pas simplement une propriété de l'ensemble du code, mais quelque chose qui existe et diffère sur plusieurs niveaux d'abstraction simultanément.
Christian Hackl
2
Je pense qu'une façon de le dire est que, en général, l'extraction d'une condition ne devrait être effectuée que s'il existe un nom significatif et non obscur pour cette condition. C'est une condition nécessaire mais non suffisante.
JimmyJames
Re "... parce que vous avez maintenant une fonction visible à partir de plusieurs points du programme" : en Pascal, il est possible d'avoir des fonctions locales - "... Chaque procédure ou fonction peut avoir sa propre déclaration de goto labels, constantes , types, variables et autres procédures et fonctions, ... "
Peter Mortensen
2
@PeterMortensen: C'est également possible en C # et JavaScript. Et c'est génial! Mais le point reste, une fonction, même locale, est visible dans une portée plus grande qu'un fragment de code en ligne.
JacquesB
9

Je ferais remarquer qu'il n'y a rien de fondamentalement faux avec ceci:

if(contact.email != null && contact.email.contains('@')

Au moins en supposant que c'est utilisé cette fois.

Je pourrais avoir des problèmes avec cela très facilement:

private Boolean isEmailValid(String email){
   return email != null && email.contains('@');
}

Quelques points à surveiller:

  1. Pourquoi est-ce privé? Cela ressemble à un bout potentiellement utile. Est-il assez utile d'être une méthode privée et aucune chance de l'utiliser plus largement?
  2. Je ne nommerais pas personnellement la méthode IsValidEmail, peut-être ContainsAtSign ou LooksVaguelyLikeEmailAddress, car elle ne fait presque aucune validation, ce qui est peut-être bon, peut-être pas ce qui est attendu.
  3. Est-il utilisé plus d'une fois?

S'il est utilisé une fois, est simple à analyser et prend moins d'une ligne, je suppose que la décision sera prise. Ce n'est probablement pas quelque chose que j'appellerais s'il ne s'agissait pas d'un problème particulier d'une équipe.

D'autre part, j'ai vu des méthodes faire quelque chose comme ça:

if (contact.email != null && contact.email.contains('@')) { ... }
else if (contact.email != null && contact.email.contains('@') && contact.email.contains("@mydomain.com")) { //headquarters email }
else if (contact.email != null && contact.email.contains('@') && (contact.email.contains("@news.mydomain.com") || contact.email.contains("@design.mydomain.com") ) { //internal contract teams }

Cet exemple n'est évidemment pas DRY.

Ou même juste cette dernière déclaration peut donner un autre exemple:

if (contact.email != null && contact.email.contains('@') && (contact.email.contains("@news.mydomain.com") || contact.email.contains("@design.mydomain.com") )

L’objectif devrait être de rendre le code plus lisible:

if (LooksSortaLikeAnEmail(contact.Email)) { ... }
else if (LooksLikeFromHeadquarters(contact.Email)) { ... }
else if (LooksLikeInternalEmail(contact.Email)) { ... }

Un autre scénario:

Vous pourriez avoir une méthode comme:

public void SaveContact(Contact contact){
   if (contact.email != null && contact.email.contains('@'))
   {
       contacts.Add(contact);
       contacts.Save();
   }
}

Si cela correspond à votre logique métier et n’est pas réutilisé, il n’ya pas de problème ici.

Mais quand quelqu'un demande "Pourquoi" @ "est-il enregistré, car ce n'est pas correct!" et vous décidez d’ajouter une validation réelle, puis extrayez-la!

Vous serez heureux de l'avoir fait lorsque vous avez également besoin de créer un compte pour le deuxième compte de messagerie du président Pr3 $ sid3nt @ h0m3! @ Mydomain.com et que vous décidez de tout mettre en œuvre pour essayer de soutenir RFC 2822.

Sur la lisibilité:

// If there is an email property and it contains an @ sign then process
if (contact.email != null && contact.email.contains('@'))

Si votre code est aussi clair, vous n'avez pas besoin de commentaires ici. En fait, vous n'avez pas besoin de commentaires pour dire ce que le code fait le plus souvent, mais plutôt pourquoi il le fait:

// The UI passes '@' by default, the DBA's made this column non-nullable but 
// marketing is currently more concerned with other fields and '@' default is OK
if (contact.email != null && contact.email.contains('@'))

Que les commentaires ci-dessus une déclaration if ou à l'intérieur d'une méthode minuscule est pour moi pédant. Je pourrais même faire valoir le contraire d'utile avec de bons commentaires dans une autre méthode, car il faudrait maintenant naviguer vers une autre méthode pour voir comment et pourquoi il fait ce qu'il fait.

En résumé: ne mesurez pas ces choses; Concentrez-vous sur les principes sur lesquels le texte a été construit (DRY, SOLID, KISS).

// A valid class that does nothing
public class Nothing 
{

}
AthomSfere
la source
3
Whether the comments above an if statement or inside a tiny method is to me, pedantic.C'est un problème de "paille qui a cassé le dos du chameau". Vous avez raison de dire que cette chose-là n'est pas particulièrement difficile à lire. Mais si vous avez une grande méthode (par exemple une grande importation) qui a des dizaines de ces petites évaluations, ayant ces encapsulé dans les noms de méthode lisibles ( IsUserActive, GetAverageIncome, MustBeDeleted, ...) deviendra une amélioration notable à la lecture du code. Le problème avec l'exemple, c'est qu'il n'observe qu'une paille, pas le paquet en entier qui casse le dos du chameau.
Flater
@Flater et j'espère que c'est l'esprit que le lecteur tire de cela.
AthomSfere
1
Cette "encapsulation" est un anti-motif, et la réponse le démontre réellement. Nous revenons pour lire le code à des fins de débogage et d’extension du code. Dans les deux cas, il est essentiel de comprendre ce que fait le code. Le bloc de code qui commence if (contact.email != null && contact.email.contains('@'))est bogué. Si if est faux, aucune des autres lignes si ne peut être vraie. Ce n'est pas du tout visible dans le LooksSortaLikeAnEmailbloc. Une fonction contenant une seule ligne de code n’est guère meilleure qu’un commentaire expliquant le fonctionnement de la ligne.
Quirk
1
Au mieux, une autre couche d'indirection obscurcit les mécanismes réels et rend le débogage plus difficile. Au pire, le nom de la fonction est devenu un mensonge de la même manière que les commentaires deviennent des mensonges - le contenu est mis à jour mais le nom ne l'est pas. Ce n’est pas une atteinte à l’encapsulation en général, mais cet idiome particulier est symptomatique du grand problème moderne que pose l’ingénierie logicielle «entreprise» - des couches et des couches d’abstraction et de colle enfouissant la logique pertinente.
Quirk
@quirk Je pense que vous êtes d'accord avec mon point de vue général? Et avec la colle, votre problème est tout à fait différent. En fait, j'utilise des cartes de code lorsque je regarde un nouveau code d'équipe. C'est effrayant ce que j'ai vu faire pour certaines grandes méthodes appelant une série de grandes méthodes, même au niveau du modèle MVC.
AthomSfere
6

Clean Code est un excellent livre, qui mérite d'être lu, mais ce n'est pas l'autorité finale sur de telles questions.

Décomposer le code en fonctions logiques est généralement une bonne idée, mais peu de programmeurs le font de la même manière que Martin: à un moment donné, la transformation de toutes les fonctions en fonctions diminue le rendement, et il peut être difficile de suivre lorsque tout le code est insuffisant. pièces.

Une option lorsqu'il ne vaut pas la peine de créer une toute nouvelle fonction consiste simplement à utiliser une variable intermédiaire:

boolean isEmailValid = (contact.email != null && contact.emails.contains('@');

if (isEmailValid) {
...

Cela permet de garder le code facile à suivre sans avoir à parcourir le fichier beaucoup.

Un autre problème est que Clean Code est en train de devenir assez vieux comme livre. Beaucoup de génie logiciel a évolué dans le sens de la programmation fonctionnelle, alors que Martin fait tout son possible pour ajouter de l’état aux choses et créer des objets. Je pense qu'il aurait écrit un livre tout à fait différent s'il l'avait écrit aujourd'hui.

Rich Smith
la source
Certains sont préoccupés par la ligne de code supplémentaire proche de la condition (je ne le suis pas du tout), mais peut-être en parler dans votre réponse.
Peter Mortensen Le
5

Compte tenu du fait que la condition "est valide pour le courrier électronique" que vous avez actuellement accepterait l'adresse de courrier électronique très invalide " @", je pense que vous avez toutes les raisons de supprimer une classe EmailValidator. Mieux encore, utilisez une bonne bibliothèque bien testée pour valider les adresses électroniques.

Les lignes de code en tant que métrique n'ont pas de sens. Les questions importantes en génie logiciel ne sont pas:

  • Avez-vous trop de code?
  • Avez-vous trop peu de code?

Les questions importantes sont:

  • L’application dans son ensemble est-elle conçue correctement?
  • Le code est-il correctement implémenté?
  • Le code est-il maintenable?
  • Le code est-il testable?
  • Le code est-il correctement testé?

Je n'ai jamais pensé à LoC lors de l'écriture de code pour une raison autre que Code Golf. Je me suis posé la question "Est-ce que je pourrais écrire ceci plus succinctement?"

Bien sûr, je pourrais peut-être utiliser une longue chaîne d'opérations booléennes au lieu d'une méthode utilitaire, mais devrais-je?

Votre question me fait penser à de longues chaînes de booléens que j'ai écrits et réalise que j'aurais probablement dû écrire une ou plusieurs méthodes d'utilité à la place.

Clément Cherlin
la source
3

D'un côté, ils ont raison: moins de code, mieux c'est. Une autre réponse a cité Gate, je préfère:

“Si le débogage est le processus de suppression des bogues logiciels, la programmation doit être le processus de leur insertion.” - Edsger Dijkstra

«Lors du débogage, les novices insèrent un code correctif. les experts enlèvent le code défectueux. »- Richard Pattis

Les composants les moins chers, les plus rapides et les plus fiables sont ceux qui ne sont pas présents. - Gordon Bell

En bref, moins vous avez de code, moins vous risquez de vous tromper. Si quelque chose n'est pas nécessaire, alors coupez-le.
Si le code est trop compliqué, simplifiez-le jusqu'à ce qu'il ne reste que les éléments fonctionnels.

Ce qui est important ici, c’est que tous se réfèrent à la fonctionnalité et ne disposent que du minimum requis pour le faire. Cela ne dit rien sur la façon dont cela s'exprime.

Ce que vous faites en essayant d’avoir un code propre n’est pas contre ce qui précède. Vous ajoutez à votre LOC mais n’ajoutez pas de fonctionnalités inutilisées.

L'objectif final est d'avoir un code lisible mais pas de suppléments superflus. Les deux principes ne doivent pas agir l'un contre l'autre.

Une métaphore serait de construire une voiture. La partie fonctionnelle du code est le châssis, le moteur, les roues ... ce qui fait fonctionner la voiture. La façon dont vous divisez cela ressemble plus à la suspension, à la direction assistée, etc., cela facilite la manipulation. Vous voulez que vos mécaniciens soient aussi simples que possible tout en effectuant leur travail, afin de minimiser les risques d'erreur, mais cela ne vous empêche pas d'avoir de belles places.

Baldrickk
la source
2

Les réponses existantes comportent beaucoup de sagesse, mais j'aimerais ajouter un facteur supplémentaire: le langage .

Certaines langues utilisent plus de code que d'autres pour obtenir le même effet. En particulier, alors que Java (dont je soupçonne qu’il est le langage dans la question) est extrêmement bien connu et généralement très solide, clair et direct, certaines langues plus modernes sont beaucoup plus concises et expressives.

Par exemple, en Java, il peut facilement prendre 50 lignes pour écrire une nouvelle classe avec trois propriétés, chacune avec un getter et un setter, et un ou plusieurs constructeurs - alors que vous pouvez accomplir exactement la même chose en une seule ligne de Kotlin * ou Scala. (Économie encore plus si vous avez aussi voulu appropriés equals(), hashCode()et toString()méthodes.)

Le résultat est qu'en Java, ce travail supplémentaire signifie que vous êtes plus susceptible de réutiliser un objet général qui ne convient pas vraiment, de compresser des propriétés dans des objets existants ou de transmettre individuellement un ensemble de propriétés "nues"; alors que dans un langage concis et expressif, il est plus probable que vous écriviez un meilleur code.

(Cela met en évidence la différence entre la complexité 'superficielle' du code et la complexité des idées / modèles / traitements qu'il implémente. Les lignes de code ne sont pas une mauvaise mesure du premier, mais ont beaucoup moins à voir avec le second. .)

Donc, le «coût» de bien faire les choses dépend de la langue. Peut-être qu'un signe d'une bonne langue est un signe qui ne vous fait pas choisir entre bien faire les choses et les faire simplement!

(* Ce n'est pas vraiment l'endroit idéal pour un plug, mais Kotlin vaut bien un coup d'oeil à mon humble avis.)

des vertiges
la source
1

Supposons que vous travaillez avec la classe Contactactuellement. Le fait que vous écriviez une autre méthode pour valider l'adresse e-mail est la preuve que la classe Contactne gère pas une seule responsabilité.

Il gère également certaines responsabilités liées au courrier électronique, qui devraient idéalement constituer sa propre classe.


Une autre preuve que votre code est une fusion de Contactet de Emailclasse est que vous ne pourrez pas tester le code de validation du courrier électronique facilement. Il faudra beaucoup de manœuvres pour atteindre le code de validation de l’e-mail avec une grande méthode avec les bonnes valeurs. Voir la méthode à savoir ci-dessous.

private void LargeMethod() {
    //A lot of code which modifies a lot of values. You do all sorts of tricks here.
    //Code.
    //Code..
    //Code...

    //Email validation code becoming very difficult to test as it will be difficult to ensure 
    //that you have the right data till you reach here in the method
    ValidateEmail();

    //Another whole lot of code that modifies all sorts of values.
    //Extra work to preserve the result of ValidateEmail() for your asserts later.
}

Par contre, si vous aviez une classe Email distincte avec une méthode de validation de l’e-mail, alors pour tester votre code de validation à l’unité, il vous suffirait de faire un simple appel Email.Validation()avec vos données de test.


Contenu bonus: MFeather parle de la synergie profonde entre testabilité et bonne conception.

Afficher un nom
la source
1

Il a été démontré que la réduction de la LOC était corrélée à une réduction des défauts, rien d’autre. En supposant que chaque fois que vous réduisez le coût de possession, vous réduisez le risque de défauts, vous tombez essentiellement dans le piège consistant à croire que la corrélation est égale à la causalité. La LDC réduite est le résultat de bonnes pratiques de développement et non de ce qui fait la qualité du code.

D'après mon expérience, les personnes capables de résoudre un problème avec moins de code (au niveau macro) ont tendance à être plus compétentes que celles qui écrivent plus de code pour faire la même chose. Pour réduire le nombre de lignes de code, ces développeurs expérimentés utilisent / créent des abstractions et des solutions réutilisables pour résoudre les problèmes courants. Ils ne passent pas leur temps à compter des lignes de code et à se débattre pour savoir s'ils peuvent couper une ligne ici ou là. Souvent, le code qu'ils écrivent est plus détaillé que nécessaire, ils en écrivent moins.

Laisse moi te donner un exemple. J'ai dû composer avec la logique des périodes et de la manière dont elles se chevauchent, si elles sont adjacentes et quels écarts existent entre elles. Quand j'ai commencé à travailler sur ces problèmes, j'avais des blocs de code qui effectuaient les calculs partout. Finalement, j'ai construit des classes pour représenter les périodes et les opérations qui calculaient les chevauchements, les compléments, etc. Cela a immédiatement supprimé de grandes bandes de code et les a transformées en quelques appels de méthodes. Mais ces classes elles-mêmes n'étaient pas du tout réduites.

En termes clairs: si vous essayez de réduire le LOC en essayant de couper une ligne de code ici ou là avec plus de lacérations, vous le faites mal. C'est comme essayer de perdre du poids en réduisant la quantité de légumes que vous mangez. Écrivez un code facile à comprendre, à maintenir, à déboguer et à réduire le niveau de possession par la réutilisation et l’abstraction.

JimmyJames
la source
1

Vous avez identifié un compromis valide

Il y a donc un compromis à faire ici, inhérent à l'abstraction dans son ensemble. Chaque fois que quelqu'un tente d'extraire N lignes de code dans sa propre fonction afin de le nommer et de l'isoler, il facilite simultanément la lecture du site appelant (en se référant à un nom plutôt qu'à tous les détails sanglants qui le sous-tendent) et plus complexe (vous avez maintenant une signification qui est enchevêtrée dans deux parties différentes de la base de code). "Facile" est l'opposé de "difficile", mais ce n'est pas un synonyme de "simple" qui est l'opposé de "complexe". Les deux ne sont pas opposés, et l'abstraction augmente toujours la complexité afin d'insérer une forme ou une autre de facilité.

Nous constatons directement la complexité accrue lorsqu'un changement dans les exigences de l'entreprise provoque la fuite de l'abstraction. Peut-être qu'une nouvelle logique serait allée plus naturellement au milieu du code pré-résumé, par exemple si le code abstrait traverse une arborescence et que vous souhaitez vraiment collecter (et éventuellement agir) une sorte d'informations pendant que vous êtes. traverser l'arbre. En attendant, si vous avez extrait ce code, il peut y avoir d'autres sites d'appels, et l'ajout de la logique requise au milieu de la méthode risque de casser ces autres sites d'appels. Vous voyez, chaque fois que nous modifions une ligne de code, il suffit de regarder le contexte immédiat de cette ligne de code; lorsque nous changeons une méthode, nous devons Cmd-F notre code source entier pour rechercher tout ce qui pourrait casser à la suite du changement de contrat de cette méthode,

L'algorithme glouton peut échouer dans ces cas

La complexité a également rendu le code, dans un certain sens, moins lisible que plus lisible.. Dans un travail précédent, je travaillais avec une API HTTP structurée de manière très précise et précise en plusieurs couches. Chaque point de terminaison est spécifié par un contrôleur qui valide la forme du message entrant, puis le transmet à un gestionnaire "couche de gestion". , qui a ensuite demandé à une "couche de données" qui était chargée de poser plusieurs requêtes à une couche "d'objet d'accès à des données", chargée de créer plusieurs délégués SQL qui répondraient effectivement à votre question. La première chose que je puisse dire à ce sujet a été que 90% du code était copié-collé, autrement dit qu’il s’agissait d’une solution simple. Ainsi, dans de nombreux cas, la lecture d’un passage de code était très "facile", car "oh ce gestionnaire ne fait que transmettre la demande à cet objet d’accès aux données".Beaucoup de changements de contexte, de recherche de fichiers et de suivi des informations que vous n'auriez jamais dû suivre ", cela s'appelle X sur cette couche, il s'appelle X 'sur cette autre couche, puis il s'appelle X' 'dans cette autre autre couche. "

Je pense que lorsque je me suis arrêté, cette simple API CRUD était au stade où, si vous l’imprimiez à 30 lignes par page, cela prendrait entre 10 et 20 manuels de cinq cents pages sur une étagère: c’était toute une encyclopédie de tâches répétitives. code. En termes de complexité essentielle, je ne suis pas sûr qu’il y ait même la moitié d’un manuel de complexité essentielle; nous n'avions peut-être que 5 ou 6 diagrammes de base de données pour le gérer. Effectuer un léger changement était une entreprise gigantesque, apprendre que c’était une entreprise gigantesque, ajouter de nouvelles fonctionnalités devenait tellement pénible que nous avions des fichiers de gabarit standard que nous utiliserions pour ajouter de nouvelles fonctionnalités.

J'ai donc constaté de première main comment rendre chaque partie très lisible et évident peut rendre le tout très illisible et non évident. Cela signifie que l'algorithme glouton peut échouer. Vous connaissez l'algorithme gourmand, oui? "Je ferai tout ce qui sera fait localement pour améliorer le plus la situation, et j'aurai ensuite confiance que je me trouve dans une situation globalement améliorée." Il s’agit souvent d’une belle première tentative mais elle peut aussi manquer dans des contextes complexes. Par exemple, dans le secteur de la fabrication, vous pouvez essayer d’accroître l’efficacité de chaque étape d’un processus de fabrication complexe - créez des lots plus volumineux, criez contre le public qui semble ne rien faire pour s’occuper de quelque chose d’autre - et cela peut souvent détruire l'efficacité globale du système.

Meilleure pratique: utilisez DRY et les longueurs pour effectuer l'appel

(Remarque: le titre de cette section est un peu une blague; je dis souvent à mes amis que quand quelqu'un dit "nous devrions faire X parce que les meilleures pratiques le disent bien ", 90% du temps, ils ne parlent pas de quelque chose comme une injection SQL ou un hachage de mot de passe ou quoi que ce soit - les meilleures pratiques unilatérales - et ainsi la déclaration peut être traduite dans 90% des cas, en "nous devrions faire X parce que je le dis ." Comme s'ils avaient peut-être un article de blog d'une entreprise qui avait fait un meilleur travail avec X plutôt qu'avec X ', mais rien ne garantit généralement que votre entreprise ressemble à cette entreprise. Il existe généralement un autre article provenant d'une autre entreprise qui a mieux fonctionné avec X' qu'avec X. Veuillez donc ne pas prendre le titre aussi. sérieusement.)

Ce que je recommanderais est basé sur une conférence de Jack Diederich intitulée Stop Writing Classes (youtube.com) . Il soulève plusieurs points intéressants dans cette discussion: par exemple, vous pouvez savoir qu'une classe n'est en réalité qu'une fonction lorsqu'elle ne comporte que deux méthodes publiques, dont le constructeur / initialiseur. Mais dans un cas, il parle de la façon dont une bibliothèque hypothétique dont la chaîne a été remplacée par "Muffin" a déclaré sa propre classe "MuffinHash" qui était une sous-classe du dicttype intégré de Python. L'implémentation était totalement vide - quelqu'un venait de penser: "nous aurons peut-être besoin d'ajouter des fonctionnalités personnalisées aux dictionnaires Python plus tard, introduisons une abstraction maintenant, juste au cas où."

Et sa réponse provocante était simplement: "nous pouvons toujours le faire plus tard, si nous en avons besoin."

Je pense que nous prétendons parfois que nous allons être de pires programmeurs dans le futur que nous ne le sommes actuellement, alors nous voudrons peut-être insérer une sorte de petite chose qui pourrait nous rendre heureux à l'avenir. Nous anticipons les besoins futurs. "Si le trafic est 100 fois plus important que prévu, cette approche ne sera pas adaptée, nous devons donc investir dès le départ dans cette approche plus dure qui sera adaptée." Très suspect.

Si nous prenons ce conseil au sérieux, nous devrons déterminer quand «plus tard» sera arrivé. La chose la plus évidente serait probablement d’établir une limite supérieure de la longueur des choses pour des raisons de style. Et je pense que le meilleur conseil qui reste serait d'utiliser DRY - ne vous répétez pas - avec ces heuristiques sur les longueurs de ligne pour corriger un trou dans les principes SOLID. Sur la base de l'heuristique de 30 lignes constituant une "page" de texte et une analogie avec la prose,

  1. Refactoriser une vérification dans une fonction / méthode lorsque vous souhaitez copier-coller. Par exemple, il existe parfois des raisons valables de copier / coller, mais vous devriez toujours vous sentir sales. Les vrais auteurs ne vous font pas relire une phrase longue et longue 50 fois dans le récit, à moins qu'ils essaient vraiment de mettre en valeur un thème.
  2. Une fonction / méthode devrait idéalement être un "paragraphe". La plupart des fonctions devraient faire environ une demi-page ou 1 à 15 lignes de code. Seulement 10% de vos fonctions devraient pouvoir s'étendre sur une page et demie, 45 lignes ou plus. Une fois que vous avez plus de 120 lignes de code et commentaires, cette chose doit être décomposée en plusieurs parties.
  3. Un fichier devrait idéalement être un "chapitre". La plupart des fichiers doivent comporter 12 pages ou moins, soit 360 lignes de code et de commentaires. Seulement 10% de vos fichiers devraient pouvoir contenir jusqu'à 50 pages ou 1500 lignes de code et commentaires.
  4. Idéalement, la plupart de votre code devrait être mis en retrait avec la ligne de base de la fonction ou à un niveau plus bas. En se basant sur certaines heuristiques concernant l’arbre source Linux, si vous êtes religieux, il ne faut peut-être que 10% de votre code au moins 2 niveaux de retrait dans la ligne de base, moins de 5% à 3 niveaux d’indentation ou plus. Cela signifie en particulier que les choses qui doivent "envelopper" une autre préoccupation, comme la gestion des erreurs dans un grand essai / attrape, doivent être retirées de la logique réelle.

Comme je l’ai mentionné plus haut, j’ai testé ces statistiques sur l’arborescence source actuelle de Linux pour trouver ces pourcentages approximatifs, mais elles tiennent également lieu de raisonnement dans l’analogie littéraire.

CR Drost
la source