Lisibilité contre maintenabilité, cas particulier d'écriture d'appels de fonction imbriqués

Mon style de codage pour les appels de fonction imbriqués est le suivant:

var result_h1 = H1(b1);
var result_h2 = H2(b2);
var result_g1 = G1(result_h1, result_h2);
var result_g2 = G2(c1);
var a = F(result_g1, result_g2);

J'ai récemment changé pour un département où le style de codage suivant est très utilisé:

var a = F(G1(H1(b1), H2(b2)), G2(c1));

Le résultat de ma méthode de codage est que, en cas de blocage d'une fonction, Visual Studio peut ouvrir le dump correspondant et indiquer la ligne où le problème se produit (les violations d'accès me préoccupent tout particulièrement.)

Je crains qu'en cas d'accident dû au même problème programmé de la première façon, je ne puisse pas savoir quelle fonction a provoqué l'accident.

D'autre part, plus vous mettez de traitement sur une ligne, plus vous obtenez de logique sur une page, ce qui améliore la lisibilité.

Est-ce que ma peur est correcte ou est-ce que je manque quelque chose, et en général, ce qui est préféré dans un environnement commercial? Lisibilité ou maintenabilité?

Je ne sais pas si c'est pertinent, mais nous travaillons en C ++ (STL) / C #.

Si vous vous sentiez obligé d'étendre un one-liner comme

 a = F(G1(H1(b1), H2(b2)), G2(c1));

Je ne t'en voudrais pas. Ce n'est pas seulement difficile à lire, c'est difficile à déboguer.

Pourquoi?

1. C'est dense
2. Certains débogueurs ne mettent en évidence que le tout en une fois
3. Il est libre de noms descriptifs

Si vous développez avec des résultats intermédiaires, vous obtenez

 var result_h1 = H1(b1);
 var result_h2 = H2(b2);
 var result_g1 = G1(result_h1, result_h2);
 var result_g2 = G2(c1);
 var a = F(result_g1, result_g2);

et c'est toujours difficile à lire. Pourquoi? Il résout deux des problèmes et en introduit un quatrième:

1. C'est dense
2. Certains débogueurs ne mettent en évidence que le tout en une fois
3. Il est libre de noms descriptifs
4. Il est encombré de noms non descriptifs

Si vous le développez avec des noms qui ajoutent une nouvelle, bonne signification sémantique, encore mieux! Un bon nom m'aide à comprendre.

 var temperature = H1(b1);
 var humidity = H2(b2);
 var precipitation = G1(temperature, humidity);
 var dewPoint = G2(c1);
 var forecast = F(precipitation, dewPoint);

Maintenant au moins cela raconte une histoire. Cela corrige les problèmes et est clairement meilleur que tout ce qui est proposé ici, mais il vous oblige à donner les noms.

Si vous le faites avec des noms sans signification comme result_thiset result_thatparce que vous ne pouvez tout simplement pas penser à de bons noms, je préférerais vraiment que vous nous épargniez le fouillis de noms sans signification et que vous l'étendiez à l'aide de bons vieux espaces:

int a = 
    F(
        G1(
            H1(b1), 
            H2(b2)
        ), 
        G2(c1)
    )
;

C’est aussi lisible, sinon plus, que celui qui porte des noms de résultats sans signification (même si ces noms de fonctions sont si géniaux).

1. C'est dense
2. Certains débogueurs ne mettent en évidence que le tout en une fois
3. Il est libre de noms descriptifs
4. Il est encombré de noms non descriptifs

Quand vous ne pouvez pas penser à de bons noms, c'est ce qu'il y a de mieux.

Pour une raison quelconque, les débogueurs aiment les nouvelles lignes . Vous devriez donc constater que le débogage n’est pas difficile:

Si cela ne suffisait pas, imaginez que l’ G2()on appelait à plus d’un endroit et que cela se produise:

Exception in thread "main" java.lang.NullPointerException
    at composition.Example.G2(Example.java:34)
    at composition.Example.main(Example.java:18)

Je pense que c’est bien que, puisque chaque G2()appel est sur sa propre ligne, ce style vous conduit directement à l’appel incriminé.

Alors s'il vous plaît, n'utilisez pas les problèmes 1 et 2 comme excuse pour nous en tenir au problème 4. Utilisez les bons noms quand vous pouvez y penser. Évitez les noms dénués de sens quand vous ne pouvez pas.

Lightness Races dans le commentaire d'Orbit souligne à juste titre que ces fonctions sont artificielles et qu'elles portent des noms pauvres morts. Voici donc un exemple d'application de ce style à un code de la nature:

var user = db.t_ST_User.Where(_user => string.Compare(domain,  
_user.domainName.Trim(), StringComparison.OrdinalIgnoreCase) == 0)
.Where(_user => string.Compare(samAccountName, _user.samAccountName.Trim(), 
StringComparison.OrdinalIgnoreCase) == 0).Where(_user => _user.deleted == false)
.FirstOrDefault();

Je déteste regarder ce flot de bruit, même lorsque l’ajout de mots n’est pas nécessaire. Voici à quoi ça ressemble dans ce style:

var user = db
    .t_ST_User
    .Where(
        _user => string.Compare(
            domain, 
            _user.domainName.Trim(), 
            StringComparison.OrdinalIgnoreCase
        ) == 0
    )
    .Where(
        _user => string.Compare(
            samAccountName, 
            _user.samAccountName.Trim(), 
            StringComparison.OrdinalIgnoreCase
        ) == 0
    )
    .Where(_user => _user.deleted == false)
    .FirstOrDefault()
;

Comme vous pouvez le constater, ce style fonctionne bien avec le code fonctionnel qui se déplace dans l'espace orienté objet. Si vous pouvez trouver de bons noms pour le faire dans un style intermédiaire, vous aurez plus de pouvoir. Jusque-là, je l'utilise. Mais dans tous les cas, s'il vous plaît, trouvez un moyen d'éviter les noms de résultats dénués de sens. Ils me font mal aux yeux.

D'autre part, plus vous mettez de traitement sur une ligne, plus vous obtenez de logique sur une page, ce qui améliore la lisibilité.

Je suis totalement en désaccord avec cela. Il suffit de regarder vos deux exemples de code pour appeler ceci comme étant incorrect:

var a = F(G1(H1(b1), H2(b2)), G2(c1));

est entendu à lire. "Lisibilité" ne signifie pas densité d'information; cela signifie "facile à lire, à comprendre et à maintenir".

Parfois, le code est simple et il est logique d’utiliser une seule ligne. D'autres fois, cela rend simplement la lecture plus difficile, sans autre avantage évident que de regrouper d'autres informations sur une seule ligne.

Cependant, je vous demanderais également de dire que "facile à diagnostiquer les accidents" signifie que le code est facile à gérer. Le code qui ne plante pas est beaucoup plus facile à maintenir. "Facile à entretenir" est obtenu principalement via le code, facile à lire et à comprendre, accompagné d’un bon ensemble de tests automatisés.

Ainsi, si vous transformez une expression unique en une expression multiligne comportant de nombreuses variables simplement parce que votre code se bloque souvent et que vous avez besoin de meilleures informations de débogage, arrêtez de le faire et rendez le code plus robuste à la place. Vous devriez préférer écrire du code ne nécessitant pas de débogage plutôt que du code facile à déboguer.

Votre premier exemple, le formulaire d'affectation unique, est illisible car les noms choisis n'ont aucune signification. Cela pourrait être un artefact de vouloir ne pas divulguer d'informations internes de votre part, le vrai code pourrait bien se passer à cet égard, on ne peut pas dire. Quoi qu’il en soit, il est long en raison de la densité extrêmement faible de l’information, ce qui ne se prête généralement pas à une compréhension aisée.

Votre deuxième exemple est condensé à un degré absurde. Si les fonctions avaient des noms utiles, cela pourrait être correct et bien lisible car il n’y en avait pas trop , mais cela confond dans la direction opposée.

Après avoir introduit des noms significatifs, vous pouvez chercher si l’une des formes semble naturelle ou s’il ya un milieu d’or à rechercher.

Maintenant que vous avez du code lisible, la plupart des bogues seront évidents, et les autres auront au moins plus de mal à vous cacher.

Comme toujours, en matière de lisibilité, les échecs sont extrêmes . Vous pouvez prendre n'importe quel bon conseil en programmation, en faire une règle religieuse et l'utiliser pour produire du code totalement illisible. (Si vous ne me croyez pas à ce sujet, découvrez ces deux gagnants IOCCC borsanyi et goren et voyez comment ils utilisent différemment des fonctions pour rendre le code totalement illisible. Astuce: Borsanyi utilise exactement une fonction, goren beaucoup, beaucoup plus ...)

Dans votre cas, les deux extrêmes sont 1) l’utilisation d’énoncés à expression unique et 2) la jonction de tout en énoncé de grande taille, concis et complexes. Quelle que soit l'approche adoptée à l'extrême, votre code est illisible.

En tant que programmeur, votre tâche est de trouver un équilibre . Pour chaque déclaration que vous écrivez, votre tâche est de répondre à la question: "Cette déclaration est-elle facile à comprendre et sert-elle à rendre ma fonction lisible?"

Le fait est qu’il n’existe pas une complexité d’énoncé mesurable qui puisse déterminer ce qu’il est bon d’inclure dans un énoncé unique. Prenons par exemple la ligne:

double d = sqrt(square(x1 - x0) + square(y1 - y0));

C'est une déclaration assez complexe, mais tout programmeur digne de ce nom devrait être capable de comprendre immédiatement ce que cela fait. C'est un modèle assez bien connu. En tant que tel, il est beaucoup plus lisible que l’équivalent

double dx = x1 - x0;
double dy = y1 - y0;
double dxSquare = square(dx);
double dySquare = square(dy);
double dSquare = dxSquare + dySquare;
double d = sqrt(dSquare);

qui brise le modèle bien connu en un nombre apparemment insignifiant de simples étapes. Cependant, la déclaration de votre question

var a = F(G1(H1(b1), H2(b2)), G2(c1));

Cela me semble trop compliqué, même s’il s’agit d’une opération en moins que le calcul de la distance . Bien sûr, cela est une conséquence directe de moi ne sachant rien F(), G1(), G2(), H1()ou H2(). Je pourrais en décider autrement si j'en savais plus. Mais c’est précisément le problème: la complexité souhaitable d’une déclaration dépend fortement du contexte et des opérations. Et vous, en tant que programmeur, êtes celui qui doit examiner ce contexte et décider quoi inclure dans une seule déclaration. Si vous vous souciez de la lisibilité, vous ne pouvez pas décharger cette responsabilité d'une règle statique.

@ Dominique, je pense que dans l'analyse de votre question, vous faites l'erreur de dire que "lisibilité" et "maintenabilité" sont deux choses distinctes.

Est-il possible d'avoir du code maintenable mais illisible? À l'inverse, si le code est extrêmement lisible, pourquoi deviendrait-il impossible à maintenir parce qu'il est lisible? Je n'ai jamais entendu parler d'un programmeur ayant joué ces facteurs l'un contre l'autre, devant choisir l'un ou l'autre!

En termes de décision d'utiliser ou non des variables intermédiaires pour les appels de fonctions imbriquées, dans le cas de 3 variables données, d'appels vers 5 fonctions distinctes et d'appels imbriqués de 3 profonds, j'aurais tendance à utiliser au moins certaines variables intermédiaires pour les décomposer, comme tu l'as fait.

Mais je ne vais certainement pas jusqu'à dire que les appels de fonction ne doivent jamais être imbriqués. C'est une question de jugement dans les circonstances.

Je dirais que les points suivants portent sur le jugement:

1. Si les fonctions appelées représentent des opérations mathématiques standard, elles sont plus susceptibles d'être imbriquées que des fonctions représentant une logique de domaine obscure dont les résultats sont imprévisibles et ne peuvent pas nécessairement être évalués mentalement par le lecteur.

2. Une fonction avec un seul paramètre est plus capable de participer à un nid (en tant que fonction interne ou externe) qu'une fonction avec plusieurs paramètres. Le fait de mélanger des fonctions d'arités différentes à différents niveaux de nidification a tendance à laisser le code ressembler à l'oreille d'un cochon.

3. Un nid de fonctions que les programmeurs ont l' habitude de voir s'exprimer d'une manière particulière - peut-être parce qu'elle représente une technique ou équation mathématique standard, qui a une implémentation standard - peut être plus difficile à lire et à vérifier s'il est décomposé en variables intermédiaires.

4. Un petit nid d'appels de fonction qui exécute une fonctionnalité simple et est déjà lisible, puis décomposé de manière excessive et atomisé, peut être plus difficile à lire qu'un appel qui n'a pas été décomposé du tout.

Les deux sont sous-optimaux. Considérer les commentaires.

// Calculating torque according to Newton/Dominique, 4th ed. pg 235
var a = F(G1(H1(b1), H2(b2)), G2(c1));

Ou des fonctions spécifiques plutôt que générales:

var a = Torque_NewtonDominique(b1,b2,c1);

Lors du choix des résultats à épeler, gardez à l'esprit le coût (copie par rapport à la référence, valeur L par rapport à la valeur r), la lisibilité et le risque individuellement pour chaque instruction.

Par exemple, déplacer de simples conversions unité / type sur leurs propres lignes n'a pas de valeur ajoutée, car elles sont faciles à lire et ont très peu de chances d'échouer:

var radians = ExtractAngle(c1.Normalize())
var a = Torque(b1.ToNewton(),b2.ToMeters(),radians);

En ce qui concerne votre souci d’analyser les vidages sur incident, la validation des entrées est généralement beaucoup plus importante. L’incident réel risque fort de se produire dans ces fonctions plutôt que par la ligne qui les appelle, et même si ce n’est pas le cas, il n’est généralement pas nécessaire de vous dire où exactement. les choses ont explosé. Il est bien plus important de savoir où les choses ont commencé à s'effondrer que de savoir où elles ont finalement explosé, ce qui correspond aux captures de validation des entrées.

La lisibilité est la majeure partie de la maintenabilité. Doute moi? Choisissez un projet volumineux dans un langage que vous ne connaissez pas (idéalement à la fois le langage de programmation et le langage des programmeurs), et voyez comment vous y prendre pour le remanier ...

Je mettrais la lisibilité entre quelque part entre 80 et 90% de maintenabilité. L’autre 10 à 20% indique à quel point il est possible de procéder à une refactorisation.

Cela dit, vous passez effectivement 2 variables à votre fonction finale (F). Ces 2 variables sont créées à l'aide de 3 autres variables. Vous auriez mieux fait de passer b1, b2 et c1 dans F, si F existe déjà, puis créez D qui fait la composition pour F et renvoie le résultat. À ce stade, il s’agit simplement de donner à D un bon nom et le style que vous utiliserez n’importera pas.

Sur un sujet connexe, vous dites que plus de logique sur la page facilite la lisibilité. C'est incorrect, la métrique n'est pas la page, c'est la méthode, et la logique MOINS qu'une méthode contient contient, plus elle est lisible.

Lisible signifie que le programmeur peut conserver la logique (entrée, sortie et algorithme) en tête. Plus il en fait, MOINS un programmeur peut le comprendre. Renseignez-vous sur la complexité cyclomatique.

Peu importe si vous êtes en C # ou C ++, tant que vous êtes dans une version de débogage, une solution possible consiste à encapsuler les fonctions

var a = F(G1(H1(b1), H2(b2)), G2(c1));

Vous pouvez écrire une expression en ligne et toujours être pointé là où le problème se pose simplement en regardant la trace de la pile.

returnType F( params)
{
    returnType RealF( params);
}

Bien sûr, si vous appelez la même fonction plusieurs fois dans la même ligne, vous ne pouvez pas savoir quelle fonction, mais vous pouvez toujours l'identifier:

• Regarder les paramètres de la fonction
• Si les paramètres sont identiques et que la fonction n’a pas d’effets secondaires, deux appels identiques deviennent deux appels identiques, etc.

Ce n'est pas une solution miracle, mais un peu moins difficile.

Sans oublier que le groupe de fonctions d'encapsulation peut même être plus bénéfique pour la lisibilité du code:

type CallingGBecauseFTheorem( T b1, C b2)
{
     return G1( H1( b1), H2( b2));
}

var a = F( CallingGBecauseFTheorem( b1,b2), G2( c1));

À mon avis, le code auto-documenté est préférable pour la maintenabilité et la lisibilité, quelle que soit la langue.

La déclaration donnée ci-dessus est dense, mais "auto-documentée":

double d = sqrt(square(x1 - x0) + square(y1 - y0));

Lorsque cassé en étapes (plus facile pour les tests, sûrement) perd tout le contexte, comme indiqué ci-dessus:

double dx = x1 - x0;
double dy = y1 - y0;
double dxSquare = square(dx);
double dySquare = square(dy);
double dSquare = dxSquare + dySquare;
double d = sqrt(dSquare);

Et évidemment, utiliser des noms de variables et de fonctions qui indiquent clairement leur objectif est inestimable.

Même les blocages "si" peuvent être bons ou mauvais en auto-documentation. Cela est grave car vous ne pouvez pas forcer les 2 premières conditions à tester la troisième ... toutes ne sont pas liées:

if (Bill is the boss) && (i == 3) && (the carnival is next weekend)

Celui-ci a un sens plus "collectif" et est plus facile à créer des conditions de test:

if (iRowCount == 2) || (iRowCount == 50) || (iRowCount > 100)

Et cette déclaration est juste une chaîne aléatoire de caractères, vue d'un point de vue auto-documenté:

var a = F(G1(H1(b1), H2(b2)), G2(c1));

En regardant la déclaration ci-dessus, la maintenabilité reste un défi majeur si les fonctions H1 et H2 modifient les mêmes "variables d'état du système" au lieu d'être unifiées en une seule fonction "H", car quelqu'un finira par modifier H1 sans même penser à la présence d'un Fonction H2 à regarder et pourrait casser H2.

Je pense qu'une bonne conception de code est très difficile car il n’existe pas de règles strictes pouvant être systématiquement détectées et appliquées.