Est-ce une bonne pratique d'utiliser des fonctions uniquement pour centraliser le code commun?

Je rencontre souvent ce problème. Par exemple, j'écris actuellement une fonction de lecture et une fonction d'écriture, et ils vérifient tous les deux s'il bufs'agit d'un pointeur NULL et que la modevariable se trouve dans certaines limites.

Il s'agit de duplication de code. Cela peut être résolu en le déplaçant dans sa propre fonction. Mais devrais-je? Ce sera une fonction assez anémique (ne fait pas grand-chose), plutôt localisée (donc pas à usage général), et ne se débrouille pas toute seule (ne peut pas comprendre pourquoi vous en avez besoin à moins de voir où elle se trouve) utilisé). Une autre option consiste à utiliser une macro, mais je veux parler des fonctions dans ce post.

Alors, devriez-vous utiliser une fonction pour quelque chose comme ça? quels sont les avantages et les inconvénients?

C'est une excellente utilisation des fonctions.

Ce sera une fonction assez anémique (ne fait pas grand-chose) ...

C'est bon. Les fonctions ne devraient faire qu'une seule chose.

plutôt localisé ...

Dans une langue OO, rendez-la privée.

(donc pas à usage général)

S'il gère plus d'un cas, il est à usage général. De plus, la généralisation n'est pas la seule utilisation des fonctions. Ils sont en effet là pour (1) vous éviter d'écrire le même code plus d'une fois, mais aussi (2) pour décomposer le code en petits morceaux pour le rendre plus lisible. Dans ce cas, il atteint à la fois (1) et (2). Cependant, même si votre fonction était appelée à partir d'un seul endroit, cela pourrait quand même aider avec (2).

et ne tient pas bien tout seul (ne peut pas comprendre pourquoi vous en avez besoin à moins de voir où il est utilisé).

Venez avec un bon nom pour lui, et il se porte bien tout seul. "ValidateFileParameters" ou quelque chose. Maintenant, tout va bien tout seul.

Cela devrait donc être totalement une fonction.

if (isBufferValid(buffer)) {
    // ...
}

Beaucoup plus lisible et plus maintenable (si la logique de vérification change un jour, vous ne la changez qu'en un seul endroit).

De plus, des choses comme celles-ci sont facilement intégrées afin que vous n'ayez même pas à vous soucier des frais généraux d'appels de fonction.

Permettez-moi de vous poser une meilleure question. Comment cela n'est-il pas une bonne pratique?

Faire la bonne chose. :)

OMI, les extraits de code valent la peine d'être déplacés vers leurs propres fonctions chaque fois que cela rend le code plus lisible , que la fonction soit très courte ou ne soit utilisée qu'une seule fois.

Il y a bien sûr des limites dictées par le bon sens. Vous ne voulez pas avoir la WriteToConsole(text)méthode avec le corps simplement Console.WriteLine(text), par exemple. Mais se tromper du côté de la lisibilité est une bonne pratique.

C'est généralement une bonne idée d'utiliser des fonctions pour supprimer la duplication dans le code.

Mais elle peut être poussée trop loin. Ceci est un appel au jugement.

Pour prendre l'exemple de votre vérification du tampon nul, je dirais probablement que le code suivant est suffisamment clair et ne devrait pas être extrait dans une fonction distincte, même si le même modèle est utilisé à quelques endroits.

if (buf==null) throw new BufferException("Null read buffer!");

Si vous incluez le message d'erreur en tant que paramètre dans une fonction de vérification nulle générique et que vous considérez également le code requis pour définir la fonction, ce n'est pas une économie LOC nette de le remplacer par:

checkForNullBuffer(buf, "Null read buffer!");

De plus, devoir plonger dans la fonction pour voir ce qu'elle fait lors du débogage signifie que l'appel de fonction est moins "transparent" pour l'utilisateur, et peut donc être considéré comme moins lisible / maintenable.

La centralisation du code est généralement toujours une bonne idée. Nous devons réutiliser le code autant que possible.

Cependant, il est important de noter comment procéder. Par exemple, lorsque vous avez un code qui compute_prime_number () ou check_if_packet_is_bad (), il est bon. Il y a de fortes chances que l'algorithme de fonctionnalité lui-même évolue et qu'il en profite.

Cependant, tout morceau de code qui se répète en prose ne peut pas être centralisé immédiatement. C'est mauvais. Vous pouvez masquer des lignes de code arbitraires à l'intérieur d'une fonction juste pour masquer un code, au fil du temps, lorsque plusieurs parties de l'application commencent à utiliser, elles doivent toutes rester compatibles avec les besoins de tous ceux qui sont appelés de la fonction.

Voici quelques questions à poser avant de poser

1. La fonction que vous créez a-t-elle sa propre signification inhérente ou est-ce juste un tas de lignes?

2. Quel autre contexte nécessitera l'utilisation des mêmes fonctions? Est-il probable que vous ayez besoin de généraliser légèrement l'API avant de l'utiliser?

3. Quelle sera l'attente des (différentes parties des) applications lorsque vous lèverez des exceptions?

4. Quels sont les scénarios pour voir que les fonctions vont évoluer?

Vous devez également vérifier s'il existe déjà des trucs comme celui-ci. J'ai vu tellement de gens tendre toujours à redéfinir leurs macros MIN, MAX plutôt que de chercher ce qui existe déjà.

Essentiellement, la question est: "Cette nouvelle fonction est-elle vraiment digne de réutilisation ou est-ce juste un copier-coller ?" Si c'est le premier, c'est bon d'y aller.

La duplication de code doit être évitée. Chaque fois que vous l'anticipez, vous devez éviter la duplication de code. Si vous ne l'aviez pas anticipé, appliquez la règle de 3: refactoriser avant que le même morceau de code ne soit dupliqué 3 fois, annotez le caprice quand il est dupliqué 2 fois.

Qu'est-ce qu'une duplication de code?

• Une routine qui se répète à plusieurs endroits dans la base de code. Cette tourine doit être plus complexe qu'un simple appel de fonction (sinon, vous ne gagnerez rien en factorisant).
• Une tâche très courante, même insignifiante (souvent un test). Cela améliorera l'encapsulation et la sémantique dans le code.

Considérez l'exemple ci-dessous:

if(user.getPrileges().contains("admin")) {
    // Do something
}

devient

if(user.isAdmin()) {
    // Do something
}

Vous avez amélioré l'encapsulation (maintenant vous pouvez changer les conditions pour être un administrateur de manière transparente) et la sémantique du code. Si un bogue est découvert dans la façon dont vous vérifiez que l'utilisateur est un administrateur, vous n'avez pas besoin d'aller jeter toute votre base de code et de faire un correctif partout (avec le risque d'en oublier un et d'obtenir une faille de sécurité dans votre application).

DRY consiste à simplifier la manipulation de code. Vous venez de toucher un petit point sur ce principe: il ne s'agit pas de minimiser le nombre de jetons dans votre code, mais plutôt de créer des points de modification uniques pour du code sémantiquement équivalent . Il semble que vos chèques aient toujours la même sémantique, ils doivent donc être placés dans une fonction au cas où vous auriez besoin de les modifier.

Si vous le voyez en double, vous devriez trouver un moyen de le centraliser.

Les fonctions sont un bon moyen (peut-être pas le meilleur mais cela dépend de la langue) Même si la fonction est anémique comme vous le dites, cela ne signifie pas qu'elle restera ainsi.

Et si vous deviez également vérifier autre chose?

Allez-vous trouver tous les endroits où vous devez ajouter la vérification supplémentaire ou simplement changer une fonction?

C'est presque toujours bon si les conditions suivantes sont remplies:

• améliore la lisibilité
• utilisé dans une portée limitée

Dans une portée plus large, vous devez soigneusement peser les compromis entre la duplication et la dépendance. Exemples de techniques de limitation de la portée: se cacher dans des sections ou modules privés sans les exposer au public.