Refactorisation de méthode longue: laisser tel quel vs séparer en méthodes vs utiliser des fonctions locales

Supposons que j'ai une longue méthode comme celle-ci:

public void SomeLongMethod()
{
    // Some task #1
    ...

    // Some task #2
    ...
}

Cette méthode n'a pas de parties répétitives qui doivent être déplacées vers une méthode distincte ou une fonction locale.

Il y a beaucoup de gens (dont moi) qui pensent que les méthodes longues sont des odeurs de code. De plus, je n'aime pas l'idée d'utiliser #region(s) ici et il y a une réponse très populaire expliquant pourquoi c'est mauvais .

Mais si je sépare ce code en méthodes

public void SomeLongMethod()
{
    Task1();

    Task2();
}

private void Task1()
{
    // Some task #1
    ...
}

private void Task2()
{
    // Some task #1
    ...
}

Je vois les problèmes suivants:

• Portée de la définition de classe polluante avec des définitions qui sont utilisées en interne par une seule méthode et qui signifient que je devrais documenter quelque part cela Task1et ne Task2sont destinées qu'aux internes de SomeLongMethod(ou toute personne qui lit mon code devra inférer cette idée).

• Saisie automatique IDE polluante (par exemple Intellisense) de méthodes qui ne seraient utilisées qu'une seule fois à l'intérieur de la SomeLongMethodméthode unique .

Donc, si je sépare ce code de méthode en fonctions locales

public void SomeLongMethod()
{
    Task1();

    Task2();

    void Task1()
    {
        // Some task #1
        ...
    }

    void Task2()
    {
        // Some task #1
        ...
    }
}

alors cela n'a pas les inconvénients des méthodes distinctes mais cela ne semble pas mieux (au moins pour moi) que la méthode d'origine.

Quelle version de SomeLongMethodest plus facile à gérer et à lire pour vous et pourquoi?

Les tâches autonomes doivent être déplacées vers des méthodes autonomes. Il n'y a pas d'inconvénient suffisamment important pour outrepasser cet objectif.

Vous dites qu'ils polluent l'espace de noms de la classe, mais ce n'est pas le compromis que vous devez considérer lors de la factorisation de votre code. Un futur lecteur de la classe (par exemple vous) est beaucoup plus susceptible de demander "Que fait cette méthode spécifique et comment dois-je la changer pour qu'elle réponde aux exigences modifiées?" que "Quel est le but et le sentiment d'être de toutes les méthodes de la classe? " (en lui faisant avoir moins de méthodes).

Bien sûr, si les tâches ne sont pas vraiment autonomes mais nécessitent le déplacement de certaines variables d'état, alors un objet de méthode, un objet d'assistance ou une construction similaire peut être le bon choix à la place. Et si les tâches sont totalement autonomes et ne sont pas du tout liées au domaine d'application (par exemple, juste le formatage de chaînes, alors il est concevable qu'elles devraient entrer dans une classe (utilitaire) complètement différente. Mais cela ne change pas le fait que " maintenir les méthodes par classe "est au mieux un objectif très subsidiaire.

Je n'aime aucune approche. Vous n'avez pas fourni de contexte plus large, mais la plupart du temps, cela ressemble à ceci:

Vous avez une classe qui fait du travail en combinant la puissance de plusieurs services en un seul résultat.

class SomeClass
{
    private readonly Service1 _service1;
    private readonly Service2 _service2;

    public void SomeLongMethod()
    {
        _service1.Task1();
        _service2.Task2();       
    }
}

Chacun de vos services ne met en œuvre qu'une partie de la logique. Quelque chose de spécial, que seul ce service peut faire. S'il a d'autres méthodes privées que celles qui prennent en charge l'API principale, vous pouvez donc les trier facilement et il ne devrait pas y en avoir trop de toute façon.

class Service1
{
    public void Task1()
    {
        // Some task #1
        ...
    }

    private void SomeHelperMethod() {}
}

class Service2
{
    void Task2()
    {
        // Some task #1
        ...
    }
}

L'essentiel est: si vous commencez à créer des régions dans votre code afin de gorup méthodes, il est temps de commencer à penser à encapsuler leur logique avec un nouveau service.

Le problème avec la création de fonctions dans la méthode est qu'elle ne réduit pas la longueur de la méthode.

Si vous voulez que le niveau de protection supplémentaire soit privé à la méthode, vous pouvez créer une nouvelle classe

public class BigClass
{
    public void SomeLongMethod();

    private class SomeLongMethodRunner
    {
        private void Task1();
        private void Task2();
    }
}

Mais les classes dans les classes sont très laides: /

La minimisation de la portée des constructions de langage telles que les variables et les méthodes est une bonne pratique. Par exemple, évitez les variables globales. Cela rend le code plus facile à comprendre et aide à éviter les abus car la construction est accessible à moins d'endroits.

Cela plaide en faveur de l'utilisation des fonctions locales.

Je suis d'accord pour dire que les méthodes longues sont souvent une odeur de code, mais je ne pense pas que ce soit le tableau d'ensemble, c'est plutôt pourquoi la méthode est longue qui indique un problème. Ma règle générale est que si le code effectue plusieurs tâches distinctes, chacune de ces tâches doit être sa propre méthode. Pour moi, il importe que ces sections de code soient répétitives ou non; sauf si vous êtes vraiment absolument sûr que personne ne voudra les appeler individuellement séparément à l'avenir (et c'est une chose très difficile à être sûre!) les mettre dans une autre méthode (et la rendre privée - ce qui devrait être un indicateur très fort vous ne vous attendez pas à ce qu'il soit utilisé en dehors de la classe).

Je dois avouer que je n'ai pas encore utilisé de fonctions locales, donc ma compréhension est loin d'être complète ici, mais le lien que vous avez fourni suggère qu'elles seraient souvent utilisées de manière similaire à une expression lambda (bien qu'il existe un certain nombre de cas d'utilisation où ils peuvent être plus appropriés que lambdas, ou où vous ne pouvez pas utiliser un lambda voir les fonctions locales par rapport aux expressions lambda pour les détails). Ici, je pense que cela pourrait être dû à la granularité des «autres» tâches; si elles sont assez triviales, alors une fonction locale serait peut-être OK? D'un autre côté, j'ai vu des exemples d'expressions lambda géantes (probablement lorsqu'un développeur a récemment découvert LINQ) qui ont rendu le code beaucoup moins compréhensible et maintenable que s'il avait été simplement appelé à partir d'une autre méthode.

La page Fonctions locales indique que:

«Les fonctions locales clarifient l'intention de votre code. Quiconque lit votre code peut voir que la méthode n'est pas appelable, sauf par la méthode conteneur. Pour les projets d'équipe, ils empêchent également un autre développeur d'appeler par erreur la méthode directement depuis un autre endroit de la classe ou du module.

Je ne suis pas sûr d'être vraiment avec MS sur celui-ci comme raison d'utilisation. La portée de votre méthode (ainsi que son nom et ses commentaires) devrait indiquer clairement quelle était votre intention à ce moment-là . Avec une fonction locale, j'ai pu voir que d'autres développeurs peuvent la voir comme plus sacro-sainte, mais potentiellement au point où si un changement survient à l'avenir qui nécessite que la fonctionnalité soit réutilisée, ils écrivent leur propre procédure et laissent la fonction locale en place ; créant ainsi un problème de duplication de code. La dernière phrase de la citation ci-dessus pourrait être pertinente si vous ne faites pas confiance aux membres de l'équipe pour comprendre une méthode privée avant de l'appeler, et ainsi l'appeler de manière inappropriée (donc votre décision pourrait en être affectée, bien que l'éducation soit une meilleure option ici) .

En résumé, en général, je préférerais la séparation en méthodes, mais MS a écrit les fonctions locales pour une raison, donc je ne dirais certainement pas de ne pas les utiliser,