Diviser le calcul de la valeur de retour et la déclaration de retour en méthodes d'une seule ligne?

J'ai eu une discussion avec un collègue sur la rupture d'une returninstruction et l'instruction qui calcule la valeur de retour sur deux lignes.

Par exemple

private string GetFormattedValue()
{
    var formattedString = format != null ? string.Format(format, value) : value.ToString();
    return formattedString;
}

au lieu de

private string GetFormattedValue()
{
    return format != null ? string.Format(format, value) : value.ToString();
}

Au niveau du code, je ne vois pas vraiment de valeur dans la première variante. Pour moi, ce dernier est plus clair, en particulier pour les méthodes aussi courtes. Son argument était que la première variante est plus facile à déboguer - ce qui est un assez petit mérite, puisque VisualStudio nous permet une inspection très détaillée des instructions, lorsque l'exécution est arrêtée en raison d'un point d'arrêt.

Ma question est, si c'est toujours un point valide pour écrire du code moins clair, juste pour faciliter le débogage un aperçu? Y a-t-il d'autres arguments pour la variante avec le calcul et l' returninstruction fractionnés ?

L'introduction de l'explication des variables est une refactorisation bien connue qui peut parfois aider à rendre les expressions compliquées plus lisibles. Cependant, dans le cas illustré,

• la variable supplémentaire "n'explique" rien qui ne soit pas clair d'après le nom de la méthode environnante
• la déclaration devient encore plus longue, donc (légèrement) moins lisible

De plus, les nouvelles versions du débogueur Visual Studio peuvent afficher la valeur de retour d'une fonction dans la plupart des cas sans introduire de variable superflue (mais attention, il y a quelques mises en garde, jetez un œil à cette ancienne publication SO et aux différentes réponses ).

Donc, dans ce cas spécifique, je suis d'accord avec vous, cependant, il existe d'autres cas où une variable explicative peut en effet améliorer la qualité du code.

Étant donné les faits suivants:

a) Il n'y a aucun impact sur le code final car le compilateur optimise la variable.

b) Le fait de le séparer améliore la capacité de débogage.

Je suis personnellement arrivé à la conclusion que c'est une bonne pratique de les séparer 99% du temps.

Il n'y a aucun inconvénient matériel à le faire de cette façon. L'argument selon lequel il gonfle le code est un terme impropre, car le code gonflé est un problème trivial par rapport au code illisible ou difficile à déboguer. De plus, cette méthode ne peut pas à elle seule créer du code déroutant, cela dépend entièrement du développeur.

Souvent, l'introduction d'une variable juste pour nommer un résultat est très utile lorsqu'elle rend le code plus auto-documenté. Dans ce cas, ce n'est pas un facteur car le nom de la variable est très similaire au nom de la méthode.

Notez que les méthodes d'une ligne n'ont aucune valeur inhérente. Si un changement introduit plus de lignes mais rend le code plus clair, c'est un bon changement.

Mais en général, ces décisions dépendent fortement de vos préférences personnelles. Par exemple, je trouve les deux solutions déroutantes car l'opérateur conditionnel est utilisé inutilement. J'aurais préféré une instruction if. Mais dans votre équipe, vous avez peut-être convenu de différentes conventions. Faites ensuite ce que vos conventions suggèrent. Si les conventions sont muettes sur un cas comme celui-ci, notez qu'il s'agit d'un changement extrêmement mineur qui n'a pas d'importance à long terme. Si ce modèle se répète, vous pouvez peut-être lancer une discussion sur la manière dont vous, en équipe, souhaitez gérer ces cas. Mais cela divise les cheveux entre «bon code» et «peut-être un tout petit peu meilleur code».

En réponse à vos questions:

Ma question est, si c'est toujours un point valide pour écrire du code moins clair, juste pour faciliter le débogage un aperçu?

Oui. En fait, une partie de votre déclaration antérieure me semble (aucune infraction) d'être un peu courte vue (voir gras ci - dessous) " Son argument que ce soit était que la première variante est plus facile à déboguer - ce qui est tout à fait un petit mérite , puisque VisualStudio nous permet une inspection très détaillée des déclarations, lorsque l'exécution est arrêtée en raison d'un point d'arrêt. "

Rendre le débogage plus facile n'est (presque) jamais de " petit mérite " car, selon certaines estimations, 50% du temps d'un programmeur est consacré au débogage ( logiciel de débogage réversible ).

Y a-t-il d'autres arguments pour la variante avec le calcul de fractionnement et l'instruction return?

Oui. Certains développeurs diraient que le calcul fractionné est plus facile à lire. Ceci, bien sûr, aide au débogage mais aide également lorsque quelqu'un essaie de déchiffrer les règles métier que votre code peut exécuter ou appliquer.

REMARQUE: les règles métier peuvent être mieux servies dans une base de données car elles peuvent changer souvent. Néanmoins, un codage clair dans ce domaine est toujours primordial. ( Comment créer un moteur de règles métier )

J'irais encore plus loin:

private string GetFormattedValue()
{
    if (format != null) {
        formattedString = string.Format(format, value);
    } else {
        formattedString = value.ToString()
    }
    return formattedString;
}

Pourquoi?

L'utilisation d'opérateurs ternaires pour une logique plus complexe serait illisible, vous utiliseriez donc un style comme celui ci-dessus pour des instructions plus complexes. En utilisant toujours ce style, votre code est cohérent et plus facile à analyser pour un humain. De plus, en introduisant ce type de cohérence (et en utilisant des codes lints et d'autres tests), vous pouvez éviter goto failles erreurs de type.

Un autre avantage est que votre rapport de couverture de code vous indiquera si vous avez oublié d'inclure un test pour formatn'est pas nul. Ce ne serait pas le cas pour l'opérateur ternaire.

Mon alternative préférée - si vous êtes dans le "obtenir un retour aussi rapide que possible" et non contre plusieurs retours d'une méthode:

private string GetFormattedValue()
{
    if (format != null) {
        return string.Format(format, value);
    }

    return value.ToString();
}

Vous pouvez donc consulter le dernier retour pour voir quelle est la valeur par défaut.

Il est cependant important d'être cohérent et de faire en sorte que toutes vos méthodes suivent l'une ou l'autre convention.

Je ne pense pas qu'une telle technique puisse être justifiée par la nécessité de déboguer. J'ai moi-même rencontré cette approche mille fois, et de temps en temps je continue de le faire, mais je garde toujours à l'esprit ce que Martin Fowler a dit à propos du débogage :

Les gens sous-estiment également le temps qu'ils passent à déboguer. Ils sous-estiment le temps qu'ils peuvent passer à chasser un long bug. Avec les tests, je sais tout de suite quand j'ai ajouté un bug. Cela me permet de corriger le bogue immédiatement, avant qu'il ne puisse ramper et se cacher. Il y a peu de choses plus frustrantes ou une perte de temps que le débogage. Ne serait-ce pas beaucoup plus rapide si nous ne créions pas les bogues en premier lieu?

Je pense que certaines personnes se bloquent sur des questions tangibles à la question, comme l'opérateur ternaire. Oui, beaucoup de gens détestent ça, alors peut-être que c'est bon de parler de toute façon.

Concernant le centre de votre question, déplacer l'instruction renvoyée pour qu'elle soit référencée par une variable ...

Cette question fait 2 hypothèses avec lesquelles je ne suis pas d'accord:

1. Que la deuxième variante est plus claire ou plus facile à lire (je dis que le contraire est vrai), et

2. que tout le monde utilise Visual Studio. J'ai utilisé Visual Studio plusieurs fois et je peux l'utiliser très bien, mais j'utilise généralement autre chose. Un environnement de développement qui force un IDE spécifique est un environnement dont je serais sceptique.

Rendre quelque chose à une variable nommée rend rarement plus difficile la lecture, cela fait presque toujours le contraire. La manière spécifique dont quelqu'un le fait peut causer des problèmes, comme si un suzerain de l'auto-documentation le fait, var thisVariableIsTheFormattedResultAndWillBeTheReturnValue = ...c'est évidemment mauvais, mais c'est un problème distinct. var formattedText = ...c'est bien.

Dans ce cas spécifique , et peut-être dans de nombreux cas puisque nous parlons de 1-liners, la variable ne vous dirait pas grand-chose que le nom de la fonction ne vous dit pas déjà. Par conséquent, la variable n'ajoute pas autant. L'argument de débogage pourrait toujours tenir, mais encore une fois, dans ce cas spécifique, je ne vois rien qui soit susceptible d'être votre objectif lors du débogage, et il peut toujours être facilement modifié plus tard si quelqu'un a besoin de ce format pour le débogage ou autre chose.

En général, et vous avez demandé la règle générale (votre exemple était juste cela, un exemple de formulaire généralisé), tous les arguments avancés en faveur de la variante 1 (2 lignes) sont corrects. Ce sont de bonnes directives à avoir. Mais les directives doivent être flexibles. Par exemple, le projet sur lequel je travaille a maintenant un maximum de 80 caractères par ligne, donc je divise beaucoup de lignes, mais je trouve généralement des lignes de 81 à 85 caractères qui seraient difficiles à diviser ou à réduire la lisibilité et je les laisse la limite.

Puisqu'il est peu probable d'ajouter de la valeur, je ne ferais pas 2 lignes pour l'exemple spécifique donné. Je referais la variante 2 (le 1-liner) car les points ne sont pas assez solides pour faire autrement dans ce cas.