La couverture de code met en évidence les méthodes inutilisées - que dois-je faire?

J'ai été chargé d'augmenter la couverture de code d'un projet Java existant.

J'ai remarqué que l'outil de couverture de code ( EclEmma ) a mis en évidence certaines méthodes qui ne sont jamais appelées de nulle part.

Ma première réaction n’est pas d’écrire des tests unitaires pour ces méthodes, mais de les mettre en évidence à mon supérieur hiérarchique et à mon équipe et de leur demander pourquoi ces fonctions sont déjà là.

Quelle serait la meilleure approche? Ecrivez des tests unitaires pour eux, ou demandez-vous pourquoi ils sont là?

1. Effacer.
2. Commettre.
3. Oublier.

Raisonnement:

1. Le code mort est mort. Par sa description même, il n’a aucun sens. Cela a peut-être eu un but à un moment donné, mais cela a disparu et le code devrait donc disparaître.
2. Le contrôle de version garantit que, dans (selon mon expérience), un événement unique et rare qui survient plus tard pour rechercher ce code peut être récupéré.
3. Comme effet secondaire, vous améliorez instantanément la couverture de code sans rien faire (sauf si le code mort est testé, ce qui est rarement le cas).

Mise en garde tirée des commentaires: la réponse initiale supposait que vous aviez bien vérifié que le code était sans aucun doute mort. Les IDE sont faillibles, et il est possible d'appeler du code qui a l'air mort. Faites appel à un expert sauf si vous êtes absolument certain.

Toutes les autres réponses sont basées sur l'hypothèse que les méthodes en question sont vraiment inutilisées. Cependant, la question ne précisait pas si ce projet est autonome ou s'il s'agit d'une bibliothèque.

Si le projet en question est une bibliothèque, les méthodes apparemment inutilisées peuvent être utilisées en dehors du projet et leur suppression briserait ces autres projets. Si la bibliothèque elle-même est vendue à des clients ou rendue publique, il peut même être impossible de localiser l'utilisation de ces méthodes.

Dans ce cas, il y a quatre possibilités:

• Si les méthodes sont private ou package-private, elles peuvent être supprimées en toute sécurité.
• Si les méthodes sont publiques, leur présence peut être justifiée même sans utilisation réelle, afin de compléter les fonctionnalités. Ils devraient être testés si.
• Si les méthodes sont publiques et inutiles, leur suppression constituera un changement radical et si la bibliothèque suit un contrôle de version sémantique , cela n'est autorisé que dans une nouvelle version majeure.
• Les méthodes publiques peuvent également être obsolètes et supprimées ultérieurement. Cela donne aux utilisateurs d'API un peu de temps pour passer des fonctions déconseillées avant leur suppression dans la prochaine version majeure.

Commencez par vérifier que votre outil de couverture de code est correct.

J'ai eu des situations où ils n'ont pas compris les méthodes appelées via des références à l'interface, ou si la classe est chargée dynamiquement quelque part.

Comme Java est compilé statiquement, il devrait être assez sûr de supprimer les méthodes. Supprimer le code mort est toujours bon. Il existe certaines probabilités qu'il existe un système de réflexion insensé qui les exécute au moment de l'exécution. Vérifiez donc d'abord auprès des autres développeurs, mais supprimez-les.

Quelle serait la meilleure approche? Ecrivez des tests unitaires pour eux, ou demandez-vous pourquoi ils sont là?

La suppression de code est une bonne chose.

Lorsque vous ne pouvez pas supprimer le code, vous pouvez certainement le marquer comme @Deprecated , en indiquant la version majeure que vous ciblez pour supprimer la méthode. Ensuite, vous pouvez le supprimer "plus tard". Entre temps, il sera clair qu’aucun nouveau code ne dépend de celui-ci.

Je ne recommanderais pas d'investir dans des méthodes obsolètes - donc pas de nouveaux tests unitaires juste pour atteindre les objectifs de couverture.

La différence entre les deux est principalement de savoir si les méthodes font ou non partie de l' interface publiée . La suppression arbitraire de parties de l'interface publiée peut être une surprise désagréable pour les consommateurs qui dépendaient de l'interface.

Je ne peux pas parler à EclEmma, ​​mais d'après mes expériences, vous devez faire attention à la réflexion. Si, par exemple, vous utilisez des fichiers de configuration de texte pour choisir les classes / méthodes auxquelles accéder, la distinction utilisé / non utilisé peut ne pas être évidente (j'ai été gravé par ce moyen avec des temps couplés).

Si votre projet est une feuille dans le graphique de dépendance, le cas de dépréciation est alors affaibli. Si votre projet est une bibliothèque, les arguments en faveur de la dépréciation sont plus justes.

Si votre société utilise une prise en pension mono , la suppression présente un risque plus faible que dans le cas d'une prise en pension multiple.

Comme indiqué par l0b0 , si les méthodes sont déjà disponibles dans le contrôle de source, leur récupération après suppression est un exercice simple. Si cela vous inquiète vraiment, réfléchissez à la manière d’organiser vos commits de manière à pouvoir récupérer les modifications supprimées si vous en avez besoin.

Si l'incertitude est suffisante, vous pouvez envisager de commenter le code plutôt que de le supprimer. C'est un travail supplémentaire dans le chemin heureux (où le code supprimé n'est jamais restauré), mais cela facilite la restauration. Mon hypothèse est que vous devriez préférer une suppression directe jusqu'à ce que vous ayez été brûlé plusieurs fois, ce qui vous donnera quelques indications sur la façon d'évaluer "l'incertitude" dans ce contexte.

question pourquoi ils sont là?

Le temps investi pour capturer la tradition n'est pas nécessairement perdu. On m'a appris à effectuer une suppression en deux étapes - tout d'abord, en ajoutant et en validant un commentaire expliquant ce que nous avons appris sur le code, puis en supprimant ultérieurement le code (et le commentaire).

Vous pouvez également utiliser quelque chose d'analogue aux enregistrements de décisions architecturales comme moyen de capturer la tradition avec le code source.

Un outil de couverture de code n'est pas omniscient, ni omniprésent. Ce n'est pas parce que votre outil prétend que la méthode n'est pas appelée qu'elle ne s'appelle pas . Il y a une réflexion, et selon la langue, il peut y avoir d'autres moyens d'appeler la méthode. En C ou C ++, il peut y avoir des macros qui construisent des noms de fonctions ou de méthodes, et l'outil peut ne pas voir l'appel. La première étape serait donc: Effectuez une recherche textuelle du nom de la méthode et des noms associés. Demandez à des collègues expérimentés. Vous pouvez trouver qu'il est réellement utilisé.

Si vous n’êtes pas sûr, mettez un assert () au début de chaque méthode "non utilisée". Peut-être qu'on l'appelle. Ou une déclaration de journalisation.

Peut-être que le code est réellement précieux. C’est peut-être un nouveau code sur lequel un collègue travaille depuis deux semaines et qu’il allait s’allumer demain. Ce n'est pas appelé aujourd'hui parce que l'appel sera ajouté demain.

Peut-être le code est-il réellement utile, partie 2: le code peut exécuter des tests d’exécution très coûteux qui pourraient permettre de détecter des problèmes. Le code n'est activé que si les choses tournent mal. Vous supprimez peut-être un outil de débogage précieux.

Fait intéressant, le pire conseil possible "Supprimer. Engager. Oublier." est le mieux noté. (Revues de code? Vous ne faites pas de revues de code? Que faites-vous pour programmer si vous ne faites pas de critiques de code?)

Selon l'environnement dans lequel le logiciel s'exécute, vous pouvez vous connecter si la méthode est appelée. Si elle n'est pas appelée dans un délai approprié, la méthode peut être supprimée en toute sécurité.

Cette approche est plus prudente que la simple suppression de la méthode et peut s'avérer utile si vous utilisez une environnement extrêmement sensible aux erreurs.

Nous nous connectons à un #unreachable-codecanal slack dédié avec un identifiant unique pour chaque candidat à supprimer, et il s'est avéré très efficace.