Quand une fonction est-elle trop longue? [fermé]

35 lignes, 55 lignes, 100 lignes, 300 lignes? Quand devriez-vous commencer à le briser? Je pose la question parce que j'ai une fonction de 60 lignes (y compris les commentaires) et que je pensais la séparer.

long_function(){ ... }

dans:

small_function_1(){...}
small_function_2(){...}
small_function_3(){...}

Les fonctions ne seront pas utilisées en dehors de la fonction long_function, rendre les fonctions plus petites signifie plus d'appels de fonction, etc.

Quand sépareriez-vous une fonction en plus petites? Pourquoi?

1. Les méthodes ne doivent faire qu'une seule chose logique (pensez à la fonctionnalité)
2. Vous devriez être capable d'expliquer la méthode en une seule phrase
3. Il doit s'adapter à la hauteur de votre écran
4. Évitez les frais généraux inutiles (commentaires qui soulignent l'évidence ...)
5. Les tests unitaires sont plus faciles pour les petites fonctions logiques
6. Vérifier si une partie de la fonction peut être réutilisée par d'autres classes ou méthodes
7. Évitez les couplages interclasses excessifs
8. Évitez les structures de contrôle profondément imbriquées

Merci à tous pour les réponses , modifiez la liste et votez pour la bonne réponse, je choisirai celle-là;)

Je refactore maintenant avec ces idées à l'esprit :)

Il n'y a pas de règles vraiment strictes pour cela. En général, j'aime que mes méthodes "ne font qu'une chose". Donc, s'il s'agit de saisir des données, puis de faire quelque chose avec ces données, puis de les écrire sur le disque, alors je diviserais la saisie et l'écriture en méthodes séparées afin que ma méthode "principale" contienne simplement le "faire quelque chose".

Mais «faire quelque chose» pourrait encore prendre quelques lignes, donc je ne suis pas sûr qu'un certain nombre de lignes soit la bonne métrique à utiliser :)

Edit: Ceci est une seule ligne de code que j'ai envoyée au travail la semaine dernière (pour prouver un point ... ce n'est pas quelque chose dont je prends l'habitude :)) - Je ne voudrais certainement pas 50-60 de ces mauvais garçons dans ma méthode :RÉ

return level4 != null ? GetResources().Where(r => (r.Level2 == (int)level2) && (r.Level3 == (int)level3) && (r.Level4 == (int)level4)).ToList() : level3 != null ? GetResources().Where(r => (r.Level2 == (int)level2) && (r.Level3 == (int)level3)).ToList() : level2 != null ? GetResources().Where(r => (r.Level2 == (int)level2)).ToList() : GetAllResourceList();

Voici une liste de drapeaux rouges (sans ordre particulier) qui pourraient indiquer qu'une fonction est trop longue:

1. Structures de contrôle profondément imbriquées : par exemple pour les boucles for 3 niveaux de profondeur ou même juste 2 niveaux de profondeur avec des instructions if imbriquées qui ont des conditions complexes.

2. Trop de paramètres définissant l'état : Par paramètre définissant l' état , je veux dire un paramètre de fonction qui garantit un chemin d'exécution particulier à travers la fonction. Obtenez trop de ces types de paramètres et vous avez une explosion combinatoire des chemins d'exécution (cela se produit généralement en tandem avec # 1).

3. Logique qui est dupliquée dans d'autres méthodes : une mauvaise réutilisation du code contribue énormément au code procédural monolithique. Une grande partie de cette duplication logique peut être très subtile, mais une fois repensée, le résultat final peut être un design beaucoup plus élégant.

4. Couplage interclasse excessif : ce manque d'encapsulation correcte conduit les fonctions à s'intéresser aux caractéristiques intimes des autres classes, donc à les allonger.

5. Surcharge inutile : les commentaires qui soulignent les classes évidentes et profondément imbriquées, les getters et les setters superflus pour les variables de classe imbriquées privées et les noms de fonction / variable inhabituellement longs peuvent tous créer un bruit syntaxique dans les fonctions associées qui augmenteront finalement leur longueur.

6. Votre écran massif de qualité développeur n'est pas assez grand pour l'afficher : en fait, les écrans d'aujourd'hui sont suffisamment grands pour qu'une fonction proche de sa hauteur soit probablement beaucoup trop longue. Mais, s'il est plus grand , c'est un pistolet fumant que quelque chose ne va pas.

7. Vous ne pouvez pas déterminer immédiatement le but de la fonction : De plus, une fois que vous avez réellement faire déterminer son but, si vous ne pouvez pas résumer cet effet dans une seule phrase ou arrive d'avoir un mal de tête énorme, cela devrait être un indice.

En conclusion, les fonctions monolithiques peuvent avoir des conséquences importantes et sont souvent le symptôme de défauts majeurs de conception. Chaque fois que je rencontre du code qui est une joie absolue à lire, son élégance est immédiatement apparente. Et devinez quoi: les fonctions sont souvent très courtes.

Je pense qu'il y a une énorme mise en garde au mantra "ne faire qu'une seule chose" sur cette page. Parfois, faire une chose jongle avec de nombreuses variables. Ne divisez pas une longue fonction en un tas de fonctions plus petites si les fonctions plus petites finissent par avoir de longues listes de paramètres. Cela ne fait que transformer une seule fonction en un ensemble de fonctions hautement couplées sans valeur individuelle réelle.

Une fonction ne doit faire qu'une seule chose. Si vous faites beaucoup de petites choses dans une fonction, faites de chaque petite chose une fonction et appelez ces fonctions à partir de la fonction longue.

Ce que vous ne voulez vraiment pas faire, c'est copier et coller toutes les 10 lignes de votre fonction longue dans des fonctions courtes (comme le suggère votre exemple).

Je suis d'accord qu'une fonction ne devrait faire qu'une chose, mais à quel niveau se trouve cette seule chose.

Si vos 60 lignes accomplissent une chose (du point de vue de vos programmes) et que les pièces qui composent ces 60 lignes ne seront pas utilisées par autre chose, 60 lignes conviendront.

Il n'y a aucun avantage réel à le briser, à moins que vous ne puissiez le diviser en morceaux concrets qui se tiennent seuls. La métrique à utiliser est la fonctionnalité et non les lignes de code.

J'ai travaillé sur de nombreux programmes où les auteurs ont poussé la seule chose à un niveau extrême et tout ce qu'il a fini par faire était de donner l'impression que quelqu'un avait pris une grenade pour une fonction / méthode et l'a fait exploser en des dizaines de pièces non connectées qui sont dur à suivre.

Lorsque vous extrayez des éléments de cette fonction, vous devez également déterminer si vous allez ajouter des frais généraux inutiles et éviter de transmettre de grandes quantités de données.

Je crois que le point clé est de rechercher la réutilisabilité dans cette longue fonction et de retirer ces pièces. Il ne vous reste que la fonction, qu'elle mesure 10, 20 ou 60 lignes.

60 lignes, c'est grand mais pas trop long pour une fonction. S'il tient sur un seul écran dans un éditeur, vous pouvez tout voir à la fois. Cela dépend vraiment de ce que font les fonctions.

Pourquoi je peux interrompre une fonction:

• C'est trop long
• Cela rend le code plus maintenable en le décomposant et en utilisant des noms significatifs pour la nouvelle fonction
• La fonction n'est pas cohérente
• Certaines parties de la fonction sont utiles en elles-mêmes.
• Quand il est difficile de trouver un nom significatif pour la fonction (cela en fait probablement trop)

Mon heuristique personnelle est que c'est trop long si je ne peux pas voir le tout sans faire défiler.

Taille environ la taille de votre écran (alors allez chercher un grand écran panoramique pivotant et tournez-le) ... :-)

Blague à part, une chose logique par fonction.

Et le point positif est que les tests unitaires sont vraiment beaucoup plus faciles à faire avec de petites fonctions logiques qui ne font qu'une seule chose. Les grandes fonctions qui font beaucoup de choses sont plus difficiles à vérifier!

/ Johan

Règle de base: si une fonction contient des blocs de code qui font quelque chose, qui est quelque peu détaché du reste du code, placez-la dans une fonction distincte. Exemple:

function build_address_list_for_zip($zip) {

    $query = "SELECT * FROM ADDRESS WHERE zip = $zip";
    $results = perform_query($query);
    $addresses = array();
    while ($address = fetch_query_result($results)) {
        $addresses[] = $address;
    }

    // now create a nice looking list of
    // addresses for the user
    return $html_content;
}

beaucoup plus agréable:

function fetch_addresses_for_zip($zip) {
    $query = "SELECT * FROM ADDRESS WHERE zip = $zip";
    $results = perform_query($query);
    $addresses = array();
    while ($address = fetch_query_result($results)) {
        $addresses[] = $address;
    }
    return $addresses;
}

function build_address_list_for_zip($zip) {

    $addresses = fetch_addresses_for_zip($zip);

    // now create a nice looking list of
    // addresses for the user
    return $html_content;
}

Cette approche a deux avantages:

1. Chaque fois que vous avez besoin de récupérer des adresses pour un certain code postal, vous pouvez utiliser la fonction facilement disponible.

2. Chaque fois que vous avez besoin de relire la fonction build_address_list_for_zip(), vous savez ce que le premier bloc de code va faire (il récupère les adresses d'un certain code postal, du moins c'est ce que vous pouvez déduire du nom de la fonction). Si vous aviez laissé le code de requête en ligne, vous devez d'abord analyser ce code.

[D'un autre côté (je vais nier que je vous ai dit cela, même sous la torture): Si vous lisez beaucoup sur l'optimisation PHP, vous pourriez avoir l'idée de garder le nombre de fonctions aussi petit que possible, car les appels de fonction sont très, très cher en PHP. Je n'en sais rien car je n'ai jamais fait de benchmarks. Si tel est le cas, vous feriez probablement mieux de ne suivre aucune des réponses à votre question si votre application est très "sensible aux performances" ;-)]

Jetez un œil à la cyclomatique de McCabe, dans laquelle il décompose son code en un graphe où: «Chaque nœud du graphe correspond à un bloc de code dans le programme où le flux est séquentiel et les arcs correspondent à des branches prises dans le programme. "

Imaginez maintenant que votre code n'a pas de fonctions / méthodes; c'est juste une énorme étendue de code sous la forme d'un graphique.

Vous voulez diviser cet étalement en méthodes. Considérez que, lorsque vous le faites, il y aura un certain nombre de blocs dans chaque méthode. Un seul bloc de chaque méthode sera visible par toutes les autres méthodes: le premier bloc (nous supposons que vous ne pourrez sauter dans une méthode qu'à un seul point: le premier bloc). Tous les autres blocs de chaque méthode seront des informations cachées dans cette méthode, mais chaque bloc d'une méthode peut potentiellement accéder à n'importe quel autre bloc de cette méthode.

Pour déterminer la taille de vos méthodes en termes de nombre de blocs par méthode, une question que vous pourriez vous poser est la suivante: combien de méthodes dois-je avoir pour minimiser le nombre potentiel maximum de dépendances (MPE) entre tous les blocs?

Cette réponse est donnée par une équation. Si r est le nombre de méthodes qui minimise le MPE du système, et n est le nombre de blocs dans le système, alors l'équation est: r = sqrt (n)

Et on peut montrer que cela donne le nombre de blocs par méthode à être également sqrt (n).

Gardez à l'esprit que vous pouvez finir par re-factoriser juste pour le ré-factoriser, rendant potentiellement le code plus illisible qu'il ne l'était au départ.

Un ancien de mes collègues avait une règle bizarre selon laquelle une fonction / méthode ne doit contenir que 4 lignes de code! Il a essayé de s'en tenir à cela de manière si rigide que les noms de ses méthodes devenaient souvent répétitifs et dénués de sens et les appels devenaient profondément imbriqués et déroutants.

Donc, mon propre mantra est devenu: si vous ne pouvez pas penser à un nom de fonction / méthode décent pour le morceau de code que vous re-factorisez, ne vous inquiétez pas.

La principale raison pour laquelle je décompose généralement une fonction est soit parce que des morceaux de celui-ci sont également des ingrédients dans une autre fonction à proximité que j'écris, de sorte que les parties communes sont prises en compte. De plus, s'il utilise beaucoup de champs ou de propriétés d'une autre classe, il y a de fortes chances que le morceau pertinent puisse être retiré en gros et si possible déplacé dans l'autre classe.

Si vous avez un bloc de code avec un commentaire en haut, envisagez de le retirer dans une fonction, avec les noms de fonction et d'argument illustrant son objectif, et de réserver le commentaire pour la justification du code.

Êtes-vous sûr qu'il n'y a pas de pièces là-dedans qui seraient utiles ailleurs? De quelle sorte de fonction s'agit-il?

À mon avis, la réponse est: quand il fait trop de choses. Votre fonction ne doit effectuer que les actions que vous attendez du nom de la fonction elle-même. Une autre chose à considérer est si vous souhaitez réutiliser certaines parties de vos fonctions dans d'autres; dans ce cas, il pourrait être utile de le scinder.

Je décompose généralement les fonctions par la nécessité de placer des commentaires décrivant le bloc de code suivant. Ce qui était auparavant dans les commentaires va maintenant dans le nouveau nom de la fonction. Ce n'est pas une règle stricte, mais (pour moi) une belle règle de base. J'aime mieux parler le code pour lui-même que celui qui a besoin de commentaires (car j'ai appris que les commentaires mentent généralement)

C'est en partie une question de goût, mais comment je le détermine, c'est que j'essaie de ne conserver mes fonctions à peu près que le temps qu'elles tiennent sur mon écran à la fois (au maximum). La raison en est qu'il est plus facile de comprendre ce qui se passe si vous pouvez tout voir en même temps.

Quand je code, c'est un mélange d'écriture de longues fonctions, puis de refactorisation pour extraire des bits qui pourraient être réutilisés par d'autres fonctions - et - d'écrire de petites fonctions qui font des tâches discrètes au fur et à mesure.

Je ne sais pas s'il y a une bonne ou une mauvaise réponse à cela (par exemple, vous pouvez vous contenter de 67 lignes comme maximum, mais il peut y avoir des moments où il est logique d'en ajouter quelques autres).

Il y a eu des recherches approfondies sur ce sujet même, si vous voulez le moins de bogues, votre code ne devrait pas être trop long. Mais cela ne devrait pas non plus être trop court.

Je ne suis pas d'accord sur le fait qu'une méthode doit tenir sur votre écran en une seule, mais si vous faites défiler vers le bas de plus d'une page, la méthode est trop longue.

Voir La taille de classe optimale pour les logiciels orientés objet pour plus de détails.

J'ai écrit des fonctions de 500 lignes auparavant, mais ce n'étaient que de grosses instructions de commutation pour le décodage et la réponse aux messages. Lorsque le code d'un seul message est devenu plus complexe qu'un seul if-then-else, je l'ai extrait.

En substance, bien que la fonction soit de 500 lignes, les régions indépendamment maintenues ont en moyenne 5 lignes.

J'utilise normalement une approche basée sur les tests pour écrire du code. Dans cette approche, la taille de la fonction est souvent liée à la granularité de vos tests.

Si votre test est suffisamment ciblé, il vous conduira à écrire une petite fonction ciblée pour que le test passe.

Cela fonctionne également dans l'autre sens. Les fonctions doivent être suffisamment petites pour être testées efficacement. Ainsi, lorsque je travaille avec du code hérité, je trouve souvent que je décompose des fonctions plus grandes afin de tester les différentes parties de celles-ci.

Je me demande généralement "quelle est la responsabilité de cette fonction" et si je ne peux pas énoncer la responsabilité dans une phrase claire et concise, puis traduire cela en un petit test ciblé, je me demande si la fonction est trop grande.

S'il a plus de trois branches, cela signifie généralement qu'une fonction ou une méthode doit être séparée, pour encapsuler la logique de branchement dans différentes méthodes.

Chaque boucle for, instruction if, etc. n'est alors pas considérée comme une branche dans la méthode appelante.

Cobertura pour le code Java (et je suis sûr qu'il existe d'autres outils pour d'autres langages) calcule le nombre de if, etc. dans une fonction pour chaque fonction et le additionne pour la "complexité cyclomatique moyenne".

Si une fonction / méthode n'a que trois branches, elle obtiendra un trois sur cette métrique, ce qui est très bien.

Parfois, il est difficile de suivre cette directive, notamment pour valider les entrées de l'utilisateur. Néanmoins, le fait de placer des branches dans différentes méthodes facilite non seulement le développement et la maintenance, mais également les tests, car les entrées des méthodes qui effectuent le branchement peuvent être analysées facilement pour voir quelles entrées doivent être ajoutées aux cas de test afin de couvrir les branches qui n'étaient pas couverts.

Si toutes les branches se trouvaient dans une seule méthode, les entrées devraient être suivies depuis le début de la méthode, ce qui entrave la testabilité.

Je suppose que vous trouverez beaucoup de réponses à ce sujet.

Je le diviserais probablement en fonction des tâches logiques exécutées au sein de la fonction. S'il vous semble que votre histoire courte se transforme en roman, je vous suggère de trouver et d'extraire des étapes distinctes.

Par exemple, si vous avez une fonction qui gère une sorte d'entrée de chaîne et renvoie un résultat de chaîne, vous pouvez diviser la fonction en fonction de la logique pour diviser votre chaîne en parties, de la logique pour ajouter des caractères supplémentaires et de la logique pour la mettre tous de nouveau ensemble comme un résultat formaté.

En bref, tout ce qui rend votre code propre et facile à lire (que ce soit simplement en vous assurant que votre fonction a de bons commentaires ou en le divisant) est la meilleure approche.

en supposant que vous faites une chose, la longueur dépendra de:

• que fais tu
• quelle langue vous utilisez
• combien de niveaux d'abstraction vous devez traiter dans le code

60 lignes sont peut-être trop longues ou parfaites. je soupçonne que cela peut être trop long cependant.

Une chose (et cette chose devrait être évidente d'après le nom de la fonction), mais pas plus qu'un écran plein de code, peu importe. Et n'hésitez pas à augmenter la taille de votre police. Et en cas de doute, refactorisez-le en deux ou plusieurs fonctions.

Prolongeant l'esprit d'un tweet d'oncle Bob il y a quelque temps, vous savez qu'une fonction devient trop longue lorsque vous ressentez le besoin de mettre une ligne vide entre deux lignes de code. L'idée étant que si vous avez besoin d'une ligne vide pour séparer le code, sa responsabilité et sa portée se séparent à ce stade.

Mon idée est que si je dois me demander si c'est trop long, c'est probablement trop long. Cela permet de créer des fonctions plus petites, dans ce domaine, car cela pourrait aider plus tard dans le cycle de vie de l'application.