Si cela vous inquiète, vous devriez lire le code et proposer des suggestions (cela semble être le moment idéal pour pousser à la révision du code!). Il peut y avoir 3000 lignes en cas de besoin. Décider que juste parce qu'il y a 3000 lignes signifie que c'est faux ou mauvais est arbitraire.

[Modifié en fonction des mises à jour]

Vous dites que la vitesse du code n'est pas pertinente. À ce stade, il semble que ce soit juste du code laid. Le meilleur plan d'action puisque vous leur avez déjà donné des suggestions (puisque vous n'êtes pas leur superviseur, etc.) serait probablement de simplement l'accepter et de passer à autre chose. Si jamais vous avez besoin de travailler sur leur code, il semble qu'il soit suffisamment divisé pour qu'il puisse être facilement divisé en plusieurs fonctions, mais tel quel, cela fonctionne et vous n'aurez peut-être jamais besoin de le toucher.

ce sur quoi vos patrons veulent que vous travailliez et faites des suggestions et des améliorations là où vous pouvez les adapter. Si vous essayez de tout réparer mal, vous voyez tout d'un coup, vous vous stresserez sans raison.

Je me concentrerais sur le problème de la maintenabilité.

Selon les circonstances, une fonction de 1000 lignes qui fait une chose et qui est bien documentée peut être plus lisible et maintenable qu'une fonction de 20 lignes qui diffère chaque décision à une pile profonde de dix appels de fonctions utilitaires sur lesquelles chacune avait des cas spéciaux greffés au fil du temps au fur et à mesure que les exigences changeaient (diatribe étrangement spécifique, je sais).

ol>

La longueur d'une fonction est souvent en corrélation avec sa facilité de compréhension, mais ce n'est pas une règle absolue.

J'ai déjà travaillé sur du code hérité où l'intégralité du site Web est traitée dans un seul fichier d'environ 100 000 lignes de code. C'est vrai. Tout sur le site se fait dans un seul fichier, une seule fonction. Il est arrivé à un point où l'ajout ou la modification de quelque chose signifiait que vous faites défiler jusqu'en bas et que vous avez simplement modifié le tampon de sortie pour changer les choses. Comme si quelqu'un disait vouloir changer une phrase, nous faisons une regex pour rechercher la phrase et la remplaçons simplement par la nouvelle phrase.

Nous sommes finalement arrivés au point où il est devenu tellement gonflé que seules quelques personnes étaient des "experts" dans la modification du tampon de sortie. Il a finalement été décidé de simplement jeter le fichier, et de refaire l'ensemble du site avec une approche moderne.

Je pense que c'est ce qui va se passer ici. Maintenez la fonction 3k, et s'il y va, lancez simplement le code. C'est ce que je ferais, plutôt que de perdre du temps à essayer de convaincre quelqu'un que quelque chose est mieux. Cela fonctionne, c'est ce qu'est l'argument et cela pourrait être vrai. Sans un patron qui connaît le code ou qui possède de bonnes compétences générales, vous n'irez probablement pas loin en essayant de convaincre votre collègue égal de changer.

Une méthode longue est certainement une odeur de code, mais elle n'indique pas de manière concluante que quelque chose ne va pas. En fait, je dirais que vous ne devriez pas séparer une méthode uniquement sur la longueur, c'est arbitraire. Par exemple, j'ai vu de longues méthodes pour des tâches ETL (Extract / Transform / Load) distinctes où la longueur est vraiment déterminée par la quantité de données.

Ne le signalez pas au patron à ce stade. Trouvez une raison tangible pour laquelle la méthode peut être améliorée, puis communiquez-la au développeur de manière constructive.

Ne vous en faites pas. Ce n'est pas votre problème (pour le moment). C'est la responsabilité des gars de maintenir le code, pas le vôtre. Ne touchez pas. Si votre patron vous demande de le toucher, que ce soit maintenant ou plus tard, dites-lui pourquoi vous ne le ferez pas.

Si votre collègue part, vous achetez un livre sur la refactorisation. C'est à ce moment que vous dites à votre patron que cette fonction n'est pas maintenable.

Comment puis-je réagir de manière professionnelle à cela?

"Génial, merci!"

Maintenant, si vous pensez que le code est sous-optimal, quel que soit le nombre de lignes , vous pouvez le tester sur le côté pour voir s'il répond à des exigences de performances. Cela vous donne des informations significatives et exploitables.

Si vous pensez que le code suit de mauvaises pratiques, vous pouvez en parler lors de la révision du code.

Vous avez dit d'une part que chaque développeur est responsable de son propre code, et pourtant vous souhaitez signaler un de vos collègues à votre responsable pour ne pas travailler selon vos standards.

Si le code fonctionne comme prévu, il n'y a rien à dire à ce sujet jusqu'à ce qu'il y ait un moment où c'est un problème ou qu'il y ait un moment où les normes de codage sont définies pour l'équipe dans son ensemble.

Demandez-lui d'écrire des tests pour prouver que cela fonctionne. Vérifiez la couverture des tests. S'il est inférieur à 100%, c'est un problème.

Ensuite, au moins, si vous devez le maintenir un jour, vous pourrez faire des changements sans craindre de foirer quelque chose sans vous en rendre compte.

Vous avez deux points ici:

• Dans votre position actuelle, ce n'est pas votre problème , vous avez dit que votre patron a demandé à ce collègue de terminer le code rapide, donc il l'a fait, laissant derrière ces 3000 lignes avec beaucoup de dette technique , vous pouvez le mentionner à votre patron (il ne comprend pas la programmation, mais le concept de dette est quelque chose que tout le monde sait) expliquez simplement que pendant que le code fonctionne, si quelque chose se brise ou si le collègue qui l'écrit quitte l'entreprise, il lui faudra du temps pour comprendre ce qu'il a fait, il comprendra peut-être mais même s'il comprend cela, il dira que ce n'est pas un problème maintenant, car ce n'est pas un problème, du moins pas le vôtre, ce qui nous amène au point suivant.

• Donc, le collègue quitte l'entreprise et est maintenant votre problème (vous mentionnez qu'il y a 3 développeurs, donc même dans ce cas cela ne pourrait pas être votre problème), eh bien, (je peux parler de mon expérience) ne le touchez pas sauf si c'est nécessaire, parlez à votre b Encore une fois, souvenez-vous de lui lors de la première discussion, mais ne commencez pas à refactoriser ce code tant que votre patron ne l'a pas approuvé ou que le s #! t n'a pas touché le ventilateur, car si vous commencez à refactoriser ce code, cela pourrait être facile mais il y a une forte probabilité qu'il vous faudra du temps pour que votre patron s'attende à se concentrer sur autre chose.

Quelques points à prendre en considération, même si c'est une petite entreprise avec un patron qui ne Je ne comprends pas comment fonctionne le développement logiciel:

• Comme le mentionne une autre réponse, bien que votre patron ne soit pas au courant du développement, il connaît les affaires (je l'espère), donc la meilleure façon de le faire comprendre pourquoi les revues de code, les tests unitaires et autres pratiques sont nécessaires, c'est de lui dire que ce sont des investissements, moins de bogues, moins de temps d'arrêt de production, etc.
• Je ne sais pas qui est le senior de votre équipe, si est celui avec les 3000 lignes, vous pourriez avoir du mal à convaincre votre patron que ce qu'il fait n'est pas la meilleure approche. Cela s'applique si vous êtes celui qui a le moins d'expérience.
• Vous voudrez peut-être parler avec votre collègue et essayer de comprendre pourquoi il a fait cela, il mentionne que c'est optimisé cela pourrait signifier que le code fonctionne, comme vous le dites, et que c'est rapide ( vous ne pensez peut-être pas que la vitesse est pertinente, mais pour les affaires, c'est parfois un gros problème) mais il sait qu'il a une dette technique et il prévoit de résoudre ce problème, il suffit de suivre les instructions de votre patron pour terminer aussi vite.

Je pense que le meilleur exemple de se faire heurter par un bus est peut-être le meilleur.

J'irais voir votre patron et lui dirais: "J'ai des inquiétudes concernant la maintenabilité des codes, si l'employé est heurté par un bus ou autrement indisponible, il me faudra ## semaines pour le découvrir. Êtes-vous d'accord avec le fait que je ne sois pas disponible pour travailler sur un autre projet pendant ## semaines si ce projet nécessite une maintenance si une autre personne n'est pas disponible? "

Laisser votre patron prendre la décision finale sur ce qui est le plus important pour ce projet "c'est fait" ou "il est maintenable"?

Pour autant que nous sachions que le patron pourrait être yay, le client devra nous payer 2x plus parce que s'il veut que les modifications soient apportées, il devra payer pour cela.

Ceci signifie que la chose la plus importante pour le patron est de savoir pour quoi le client va payer.

Prévenir les problèmes coûte moins cher que d'attendre qu'ils surviennent et de les résoudre. Votre patron aime pas cher.

Demandez à votre patron s'il s'attend à ce que votre code soit utilisé pendant une longue période et si des changements sont probables si les clients paient pour eux.

Probablement Si vous obtenez un oui pour les deux, suggérez que vous aimeriez que le nouveau code que vous écrivez soit examiné par vos pairs. Cela ne leur prendra pas longtemps et le temps supplémentaire sera payé dix fois, car les erreurs sont beaucoup moins chères à corriger au fur et à mesure qu'elles sont détectées. Une erreur trouvée dans un code qui est frais dans l'esprit est plus facile à corriger qu'une erreur trouvée par le client avec les rapports d'erreurs moins qu'utiles habituels des clients. Assurez-lui que vous ne demanderez pas beaucoup de révision de code, peut-être une fois par semaine. S'il vous demande si vous n'êtes pas sûr de la qualité de votre code, assurez-lui que ce n'est pas du tout le cas. Mais 6 yeux ont une vision plus large que 2 et la révision du code est une pratique standard de l'industrie car ses avantages l'emportent de loin sur les coûts minimaux.

S'il l'accepte, lorsque vos amis trouvent des erreurs dans votre code, assurez-vous de le mentionner à votre patron. Mentionnez combien de temps supplémentaire vous auriez dû passer à déboguer le problème si le code était entré en production. Ou la perte de confiance des clients. Mentionnez à quel point le travail de cette équipe améliore le produit même si vous travaillez sur différents projets.

S'il passe à cette étape, il deviendra facilement la suivante: faire réviser le code de tout le monde.

S'il n'est pas disposé à accepter que quelqu'un se porte volontaire pour faire réviser son code, il n'y a aucune chance que vous l'ameniez à vous laisser réviser le code de votre ami.

D'après ce que vous décrivez, vous avez une montagne à gravir et une équipe pour la remonter.

Je ne pense pas que je parlerais spécifiquement de la méthode de la ligne 1k, je commencerais par apporter mettre en place les meilleures pratiques avec votre patron dans un 1: 1. Demandez-lui si l'équipe a des directives de codage ou des meilleures pratiques à suivre. En supposant que la réponse est non, rassemblez des liens vers des articles sur les meilleures pratiques pour le langage de programmation que vous utilisez. J'essaie de rester avec les guides de codage des grandes entreprises ... des entreprises dont tout le monde aura entendu parler comme Google, Microsoft, etc. et commencez par leurs guides de codage.

Apportez-les à votre patron avec quelques articles sur comment la mise en œuvre des meilleures pratiques aide ... quels sont les avantages, etc. N'apportez pas votre message, vous êtes le messager. Vous apportez de bonnes nouvelles sur les moyens d'être plus efficace, d'économiser de l'argent, d'avoir moins de défauts, et la liste est longue ...

Je pense que votre patron réagirait mieux à cette approche. Ensuite, une fois que vous l'avez accroché (espérons-le), vous avez une discussion d'équipe à leur sujet et commençons à les suivre. (Je voudrais ajouter quelques bonnes pratiques procédurales comme les révisions de code et autres, pas seulement des directives de codage.) Ensuite, si vous pouvez les amener à acheter jusqu'ici, alors vous commencez à appliquer ces directives et à examiner le nouveau code (et l'ancien code comme vous le rencontrez).

"Hé, j'ai trouvé cette grande méthode qui selon les meilleures pratiques ABC, nous devrions nous séparer en méthodes plus petites qui ont une seule responsabilité, etc." et partez de là.

Pour être honnête, je doute que vous iriez loin avec tout cela, mais c'est ainsi que je l'aborderais.

Si le patron ne sait rien de la programmation, je ne prendrais pas la peine de lui dire. Au son de celui-ci, votre collègue le fera pour lui-même, d'autant plus qu'il se vante de vous.

Le code doit fonctionner, c'est tout ce dont votre patron se soucie. Bien sûr, la taille et la vitesse sont des facteurs importants s'il s'agit d'un grand projet, mais s'il est de petite ou moyenne taille, le travail suffit.

Demandez à votre collègue de vous montrer le code en cours d'exécution ou le code lui-même pour que vous puissiez le vérifier.

Hormis le "3000 LOC", il y avait 3 autres phrases dans le PO qui ont attiré mon attention:

• Dois-je communiquer cela au patron, qui ne sait rien de la programmation ?
• Ma plus grande préoccupation ici est que mon collègue puisse mettre fin à la relation avec l'entreprise à un moment donné et que je devrai m'occuper de cette partie du projet sur lequel il travaillait.
• Les revues de code n'existent pas ici.

Le départ de quelqu'un est un risque commercial, peut-être un risque facile à comprendre pour un non-programmeur, et un risque atténué par des revues de code.

Vous pourriez dire à votre patron que les programmeurs devraient (au minimum) comprendre le travail de chacun - ce qu'il fait et comment il est implémenté - au cas où l'un d'eux tomberait sous un bus. Vous pouvez ajouter que c'est normal / professionnel.

Ensuite, demandez à votre collègue, lors d'une révision de code, "comment ça marche?"

Vous avez dit que votre inquiétude était ...

Comment diable puis-je lire une méthode de 3000 lignes?

... donc un examen du code devrait expliquer cela - c'est-à-dire comment ils expliquent, comment ils le lisent .

Vous voudrez peut-être insister "vous devriez utiliser des sous-programmes", mais si le collègue est hostile au changement (attaché à l'implémentation existante), il est peut-être inutile d'insister. Assurez-vous simplement de le comprendre, afin de pouvoir le modifier si vous le deviez (par exemple, si vous en héritez).

Il y a d'autres avantages aux révisions de code ...

• Détection de bogue (mais vous repérez un bogue lors d'une inspection de code)
• Transfert de connaissances (vous pourriez apprendre les uns des autres en lisant le code de l'autre et en en discutant)
• Meilleure intégration entre les modules (voir également la loi de Conway)

J'essaye d'utiliser un analyseur statique pour ce faire. Les commentaires d'un ordinateur ne peuvent pas être faux, et de cette façon, vous n'avez pas à confronter personnellement vos collègues. Il vous suffit de vous mettre d'accord sur les règles de code ou de prendre un ensemble de règles de base existantes.

Je sens deux défauts majeurs ici:

• Proposez-vous une solution au problème? Le simple fait de dire "ce code est horrible" ne sera pas travail. Cela pourrait être une opinion, cela n'apporte pas de valeur à l'entreprise; les patrons et les entreprises veulent des solutions.

• avez-vous un processus de révision? Les examens par les pairs du code doivent être effectués quelle que soit la taille de l'équipe: fournir des informations détaillées sur la façon dont le code peut être refactorisé est préférable à toute réclamation possible, et résout également le problème ci-dessus.

D'après vos modifications, la réponse à la la seconde est "non", alors peut-être que la bonne chose à signaler au patron est de parler du processus et de ne pas signaler les "échecs de code" uniques qu'il n'est pas en mesure d'évaluer correctement, à moins qu'il vous fait confiance.

Si vous dites au patron:

Écoutez, j'ai une idée pour rendre notre travail plus productif: nous pouvons écrire un meilleur code, mieux nous adapter changer dans l'entreprise, réduire les bogues et avoir une base de code maintenable qui est plus facile à saisir également pour les nouveaux arrivants

peut-être qu'il écouterait plus attentivement. Vous proposez des solutions, sans signaler un problème.

Qu'est-ce que la fonction essaie de faire? Quelle est la complexité de cette tâche? Quelle catégorie d'optimisation visaient-ils (vitesse, précision, convivialité)? Sans aucune de ces informations, il sera difficile de porter un jugement. Vous pouvez toujours consulter le code. S'il y a des inefficacités, indiquez-les comme «Le code est bon! Quickie [gain de temps / booster de performances / peu importe], vous pouvez toujours faire X pour éviter [goulot d'étranglement] '

Vous pourrez peut-être convaincre votre collègue avec des raisons techniques que le compilateur peut intégrer pour lui, donc cette "optimisation" au niveau de la source n'est pas utile.

J'espère que vous exagérez ce qu'ils ont dit à propos de la méthode de 3000 lignes la plus optimisée possible , ou bien votre collègue n'a probablement aucune idée de l'optimisation des performances et ne se soucie que de quelque chose qu'il a lu une fois. Les personnes qui pensent savoir quelque chose mais ne comprennent pas vraiment sont parfois les plus difficiles à convaincre. J'ai eu plusieurs échanges dans des commentaires de débordement de pile avec des gens refusant de croire qu'ils avaient tort, mais incapables de donner une explication technique cohérente qui avait du sens.

En tant qu'expert en optimisation asm (SO badges or dans [x86], [assembly], [performance], [sse] tags, etc.), je peux vous dire qu'il est presque impossible que cette fonction soit "la plus optimisée possible", même si votre collègue a passé des années à la profiler et à la régler ( sur un matériel spécifique? avec un système d'exploitation et une version de compilateur spécifiques?) Une fonction aussi grande aura toujours de la place pour de petits ajustements (ou de nouvelles idées pour de grands changements) qui pourraient la rendre plus rapide, ou plus petite (code machine) à la même vitesse (peut-être plus conviviale pour l'hyperthreading pour faire le même travail avec moins d'instructions) .

Je ne pense pas que le compilateur C # + JIT soit si mauvais qu'il ne puisse pas appeler la méthode en ligne pour vous, surtout s'ils n'ont qu'un seul site d'appel . Je ne connais pas C # (principalement C et C ++), mais a-t-il quelque chose qui ressemble à une fonction non membre statique en ligne que le compilateur peut intégrer au lieu d'émettre un stand -alone définition pour, et le fera même si la fonction est large? Ou quelque chose comme GNU C __attribute __ ((always_inline)) ? Votre collègue pourrait les utiliser pour avoir l'impression de bénéficier de l'optimisation qu'il juge importante sans que la source ne soit en désordre.

Mais plus important encore, "être optimisé" ne vaut un compromis en termes de lisibilité que lorsque la version de base simple (que vous avez écrite comme point de départ et pour comparer la version optimisée) est plus lente que vous le souhaitez fort>. Vous ne pouvez pas dire si vous optimisez réellement quoi que ce soit si vous n'avez pas de point de départ à comparer, et ainsi juger de la lisibilité ou de la taille du code machine / de l'empreinte du cache d'instructions par rapport à l'accélération.

Écrire une version "optimisée" moins lisible sans une base de référence simple est généralement une erreur , à moins que vous ne pensiez déjà savoir par expérience comment la version simple se compilerait et qu'elle ne serait pas efficace assez. Habituellement, vous avez une version simple dans le cadre d'un test unitaire pour une version déroulée / vectorisée manuellement. (Ce cas d'inclusion manuelle est peut-être différent, cependant. Cela ne rend pas un élément de logique individuel plus complexe ou "étrangement" implémenté. Ou est-ce le cas? Y a-t-il une optimisation manuelle entre les blocs?)

Inlining Cela en vaut souvent la peine pour les petites fonctions, mais appeler un gros bloc de code à partir de plusieurs sites d'appels n'utilise qu'une seule fois l'empreinte du cache d'instructions. Cependant, cela paie les frais généraux d'appel de fonction à chaque fois, donc le microbenchmarking d'une seule fonction, sans le contexte complet du programme, peut donner une belle apparence à l'inlining et au déroulement excessifs. Habituellement, nous allons bien laisser la décision aux heuristiques modernes du compilateur; ils sont généralement assez bien réglés, surtout s'ils peuvent effectuer une optimisation guidée par profil pour trouver des boucles qui sont réellement chaudes. (Les compilateurs JIT fonctionnent au moment de l'exécution, ils ont donc des données de profilage s'ils souhaitent les utiliser. Habituellement, en créant une version pas entièrement optimisée ou en interprétant au début, puis en utilisant des données de profilage pour des méthodes virtuelles en ligne spéculatives et des choses comme ça. / p>

Parfois, l'optimisation ne nuit pas à la lisibilité, mais dans ce cas, c'est clairement le cas.

En C ++, j'écris souvent de petites fonctions d'aide static inline qui seront intégrées dans une fonction plus grande dont j'optimise la merde avec les intrinsèques SIMD. Quand je regarde l'asm généré par le compilateur, il n'y a exactement aucun inconvénient dans l'efficacité du code machine, et un bon avantage dans la lisibilité de la source. Aucune définition autonome n'apparaît nulle part dans l'exécutable pour ces fonctions d'assistance, donc elles ne gonflent même pas l'exécutable.

Si vous voulez insister sur le problème, demandez à votre collègue s'il a a regardé la sortie asm du compilateur JIT pour sa méthode et l'a profilée, et a trouvé que ces gros blocs conditionnels permettaient au compilateur d'optimiser d'une manière qu'il ne pouvait pas avec l'inlining.

Être conscient de ce que votre compilateur + le matériel peut faire efficacement n'est pas toujours une mauvaise chose, si vous laissez cela informer vos choix de codage quand cela ne nuit pas à la lisibilité.

Il est tentant de se laisser entraîner dans l'optimisation de quelque chose qui n'a pas besoin d'être optimisé . Surtout si vous ne pensez à optimiser pour la vitesse de cette fonction que si elle a été appelée dans une boucle chaude , alors qu'elle ne sera pas dans une boucle chaude. S'il est appelé rarement, le cache de code peut être froid, donc compact est préférable. (Moins de lignes de code de cache à charger à partir de la mémoire principale.)

Ce type d'argument n'aide que si vous pouvez exclure toutes les fonctions d'assistance courantes de cette méthode de 3000 lignes. Mettre chaque bloc dans une fonction distincte ne réduira pas le code machine. Cela pourrait rendre la logique de décision pour la fonction à distribuer plus localisée, ce qui entraînerait une empreinte de cache I plus petite pour cela. Et peut-être moins de 4k pages touchées / chargées à partir du disque / i-TLBfootprint.

Quelques réflexions:

• Votre responsable pourrait éventuellement fournir la résolution des conflits et la médiation, mais il ne peut pas vraiment se prononcer sur les mérites techniques, il devra donc compter sur votre compétence. Si cela dépend de votre expertise, alors je pense que vous pouvez résoudre ce problème.
• Vous vous inquiétez des conséquences de laisser le code seul. Si les conséquences sont des coûts de maintenance, quelle meilleure façon pour votre collègue d'apprendre cette leçon que de devoir travailler dur la prochaine fois qu'il doit changer de fonction? L'expérience peut être un bon enseignant.
• Les avantages réels de la réduction des coûts sont la lisibilité, la testabilité et la simplicité. Si votre collègue ne comprend pas ces avantages, leur demander de le briser créera des limites de code artificielles. Leur montrer une meilleure façon sera utile.

Le regarder différemment: le leadership

À mon avis , votre manager n'étant pas un leader technique, aller vers lui pour résoudre ce problème réduira au minimum l'opportunité pour votre collègue d'apprendre et risquera de créer un fossé entre vous et lui / elle.

Si vous ne le faites pas demandez à un responsable technique de faire remonter cela, puis soyez ce leader en aidant votre coéquipier à devenir un meilleur développeur.

Nous pouvons résumer tout cela en deux approches possibles:

Laissez-le faire

Laissez-les faire à leur manière et lorsque ce code devient un problème, aidez-les à comprendre pourquoi c'est un problème et aidez-les à trouver quelques blocs de code clé qu'ils peuvent séparer. Ils apprendront. Sérieusement, ça va. J'ai fait cela dans des rôles de leadership. C'est juste du code. Il peut être modifié ultérieurement.

Montre le chemin

Montrez-leur une alternative avec tous les avantages réalisés . Soit réécrire leur code dans une branche en le décomposant dans les bonnes couches, rendre le code limpide (bonnes fonctions et noms de variables) et écrire de très bons tests unitaires. Ecrire des tests qui auraient été impossibles à écrire avec l'ancien code. Lorsque vous présentez ce code à votre collègue, proposez une modification arbitraire des exigences que vous voulez prétendre apporter, expliquez-leur la mise en œuvre afin qu'ils puissent voir à quel point il est facile de l'implémenter et s'assurer que cela fonctionne .