Démontrer un mauvais code au client?

Un client m'a demandé de refondre son site Web, une application de formulaires Web ASP.NET développée par un autre consultant. Cela semblait être un travail relativement simple, mais après avoir examiné le code, il est clair que ce n'est pas le cas.

Cette application n'a pas été bien écrite. Du tout. Il est extrêmement vulnérable aux attaques par injection SQL, la logique métier est répartie dans l’ensemble de l’application, il y a beaucoup de duplication et un code sans issue qui ne fait rien. En plus de cela, il continue à lancer des exceptions qui sont en train d'être étouffées, de sorte que le site semble fonctionner correctement.

Mon travail consiste simplement à mettre à jour le code HTML et CSS, mais une grande partie du code HTML est générée dans la logique métier et serait un cauchemar à résoudre. Mon estimation de la refonte est plus longue que ce que le client visait. Ils demandent pourquoi si longtemps.

Comment puis-je expliquer à mon client à quel point ce code est mauvais? Dans leur esprit, l'application fonctionne très bien et la refonte devrait être rapide et ponctuelle. C'est ma parole contre le consultant précédent. Comment puis-je donner des exemples simples et concrets qu'un client non technique comprendra?

Mise à jour

Merci pour toutes les réponses. La démonstration de l'attaque par injection SQL a du sens et je vais la démontrer dans un environnement de test. Ce n'est qu'une partie de nombreux problèmes dans cette application. Je cherchais des moyens d'expliquer pourquoi d'autres parties (telles que le code HTML généré dans la couche de données) devraient être remplacées par de meilleures pratiques pour que la mise à jour HTML et CSS ait lieu. Il y a beaucoup de bonnes suggestions ici que je vais rassembler quand je parlerai avec mon client.

Les non-techniciens ne sont pas des idiots (pour la plupart). Ils peuvent comprendre un argument technique si vous le maintenez suffisamment haut placé. Choisissez une tâche qui, à votre avis, devrait être simple et expliquez-leur pourquoi elle ne l’est pas.

Je m'attendais à ce que ce changement soit un mot dans un fichier. L’endroit le plus susceptible de le changer semblait être ici, mais lorsque je l’ai changé là-bas, cela ne fonctionnait qu’à un endroit et les 7 autres endroits s’étaient brisés. Lorsque j'en ai corrigé un, il s'est cassé deux autres places, ce qui a provoqué un effet domino. Un changement dont je pensais qu'il aurait dû prendre 10 minutes a fini par prendre 2 heures. Ce n'est qu'un exemple. Il y a beaucoup plus de tâches inattendues de 2 heures.

La structure du code, le style, la dette technique sont une chose qui - du moins au début, jusqu'à ce que le client vous fasse confiance - vous allez devoir vivre avec.

Les vulnérabilités de sécurité sont une autre affaire.

Personnellement, je ferais une estimation basée sur le travail requis en utilisant la structure et le style existants tout en précisant que la base de code pose des problèmes importants. Je soulèverais les implications en matière de sécurité séparément: faites une démonstration d'un piratage de la base de données pour ramener le message à la maison lors d'une réunion.

J'étais très heureux de le faire avec un ancien client disposant d'un système de cartes-cadeaux de fidélité lorsque j'ai mis 5 000 £ sur "ma" carte et que je le vérifiais dans sa caisse.

Quelques bonnes suggestions ici sur la façon de transmettre et de communiquer cela au client. J'espère qu'ils vont payer pour vous.

Drapeau rouge majeur ici!

Si le client vous demande de ne pas apporter de modifications autres que celles que vous avez acceptées (HTML et CSS), je transmettrais ce projet et retirerais ma soumission.

Même avec un aperçu écrit et bien communiqué de toutes les failles et des problèmes de sécurité, la responsabilité potentielle est tout simplement trop lourde pour que je sois à l'aise. Même si le client n'a jamais pris de mesure juridique ni réclamé de correctifs après un piratage ou une violation; votre nom et votre réputation sont toujours attachés à l'œuvre!

Vous risquez de perdre beaucoup plus que ce que vous pouvez gagner.

Expliquez et éventuellement démontrez le défaut
Lorsque vous en donnez la parole, tout ce que vous dites pourrait ne faire que de l'air chaud en ce qui les concerne. Une fois que vous leur avez montré comment leur application peut être utilisée de manière abusive via l'injection SQL, vous êtes soudain devenu une personne digne de confiance. Vous aurez besoin de crédibilité pour pouvoir renégocier. Et cela suffit pour changer la donne.

Être charitable à l'égard de votre prédécesseur
Cela ne veut pas dire prétendre que les erreurs ne sont pas là, mais si vous êtes condescendant, vous perdez votre crédibilité. Ne dites pas un mot sur le programmeur, sauf peut-être pour lui donner le bénéfice du doute. Concentrez-vous sur le code, pas sur le codeur. En leur faisant sentir que vous êtes le "bon gars", vous aurez beaucoup plus de marge de manœuvre dans les négociations. Et les "bons" ne disent jamais de mauvaises choses. Lorsque j'explique des erreurs de sécurité existantes (telles que des vulnérabilités d'injection SQL) au client, je préfère dire quelque chose comme ceci:

La sécurité des applications Web est un domaine en évolution rapide. De nombreux outils et techniques de développement que les gens apprennent encore aujourd'hui ont évolué avant que la plupart de ces exploits ne soient bien compris. Pour rester en avance sur les développements en matière de sécurité, vous devez suivre de très près le terrain et même parfois changer votre style de développement. La plupart des programmeurs ne le font pas.

Nous y voilà. Pas un mot du mal a parlé du développeur; il est juste "la plupart des programmeurs" ce qui signifie qu'il est en très bonne compagnie. Et maintenant, vous avez démontré que vous n'êtes pas "la plupart des programmeurs", ce qui vous donne un peu plus de crédibilité et peut-être une raison pour qu'ils vous paient plus d'argent.

Négocier un nouvel arrangement
Une fois que le client a compris que son application pouvait faire l'objet d'abus de la part du public, il souhaitait qu'elle soit corrigée. Vous êtes probablement la personne qu'il va demander de réparer. Vous pouvez vouloir ou non ce travail, alors réfléchissez bien avant de leur parler.

À tout le moins, vous voulez plus de temps pour terminer le travail qu’ils vous ont déjà confié. Vous les avez suffisamment mis au dépourvu avec la vulnérabilité pour qu’ils ne vous retiennent probablement pas à votre estimation initiale. Mais assurez-vous que le client sait ce que vous êtes et ne va pas régler dans le cadre de cet arrangement.

Généralement, le développeur (vous) préférerait refaire le tout à partir de zéro. Et dans des cas comme celui-ci, cela pourrait même être une option. Mais même dans ce cas, le client voudra quelque chose qui permettra à son entreprise de fonctionner jusqu'à la création de la nouvelle application. Cela signifie que même si vous commencez plus, vous êtes probablement encore devoir mettre à jour l'ancienne application un peu.

J'ai commencé cela par un commentaire, car au début, je pensais que c'était un aparté, mais ce n'est probablement pas le cas.

Je documenterais entièrement tout ce qui, selon vous, devrait être repensé, et pourquoi (que se passe-t-il s'ils ne font pas le changement), ainsi qu'une estimation pour résoudre le problème. Je serais particulièrement méticuleux avec tout ce que vous percevez comme un risque pour la sécurité.

Je le ferais avant de toucher un code quelconque et je m'assurerais que votre client dispose d'une copie de ce rapport, de préférence avec une sorte d'horodatage. Cela peut prendre un certain temps, mais cela vous couvrira également si l'un de ces risques de sécurité se concrétise. Encore mieux si vous pouvez faire signer quelque chose qui dit qu'ils ont reçu le document.

Bien sûr, vous pouvez pointer sur le contrôle de source du code original dont vous avez hérité si cela se produit, mais il sera beaucoup plus facile de pointer sur ce document et de dire, de manière plus professionnelle, "Vous voyez? Je vous l'avais bien dit."

Ce document peut constituer le point de départ de discussions ultérieures. Il peut même être utilisé par votre client pour que les "bonnes personnes" donnent l’autorisation nécessaire pour effectuer tout ou partie des modifications.

Cela étant dit, une fois que le client a compris les risques, souriez et supportez-le s'il lui dit de faire le travail de toute façon ou de s'en aller.

N'oubliez pas que le client s'adresse à vous pour maintenir son application. En tant que professionnel, votre travail consiste à signaler les problèmes que vous rencontrez lors de leur candidature. Le client n'a probablement aucune idée de l'existence de ces problèmes et devrait en être informé. Expliquez ces problèmes de manière à ce qu'ils puissent comprendre et laissez-les décider de la façon dont ils veulent procéder.

Utilisez des exemples concrets pour illustrer ces problèmes, tels qu'une voiture en panne ou une machine à laver nécessitant une réparation. Point est d'utiliser des exemples avec lesquels ils sont déjà familiers. Pour expliquer l'injection SQL, je voudrais simplement démontrer ce que c'est et pourquoi c'est un problème.

En fin de compte, vous souhaitez faire savoir que le succès de l'application sur laquelle vous êtes invité à travailler vous tient à cœur.

J'aime utiliser des analogies auxquelles le client peut s'identifier. La somme de travail que j'ai mise au départ pour gagner cet emploi dépendrait de la somme d'argent que le client avait l'intention de dépenser (100 $, ce qui est très différent de 20 000 $). Remarquez que j'ai dit "l'intention". Votre estimation personnelle de la valeur impliquée ne signifie pas grand chose si vous n'obtenez pas ce que vous demandez.

Dans votre cas - encore une fois en fonction de l'argent - je pourrais dessiner une boîte avec une ligne sortant de chaque côté et dire au client "Voici comment vous visualisez le logiciel maintenant. Les données vont d'un côté et sortent de l'autre, tout semble belle et propre et simple ". "C’est à quoi ressemble le logiciel à l’intérieur", puis tracez une troisième ligne reliant les deux lignes à l’intérieur de la boîte.

Ensuite, je dessinais une autre boîte, tout comme la première, avec les lignes d'entrée et de sortie à l'extérieur, sauf que cette fois, je dirais "voici à quoi ressemble vraiment le logiciel à l'intérieur de la boîte." et puis pour relier les deux lignes cette fois, je dessinais une pile aléatoire de gâchis de spaghettis, avec éventuellement des cassures, des jointures et des gribouillis.

Enfin, je dirais: "Maintenant, ce que vous me demandez, c'est ceci ..." et dessinez une forme simple à l'intérieur de la première case, peut-être un petit demi-cercle touchant la ligne, puis dites "mais pour faire ça, je ' Je dois faire cela ... "et dessiner une tornade qui ressemble à une forme de spirale autour de la ligne et continuer ..." afin de contourner tout cela ..... "et pointer les spaghettis dans l'autre boîte.

Je pense que cela ramènerait le clou dans environ 2 minutes. Si ils insistent pour que vous le fassiez quand même, alors documentez-le comme d'autres le mentionnent ci-dessus.

Comment puis-je expliquer à mon client à quel point ce code est mauvais?

Vous pouvez peut-être utiliser une analogie comme celle de plomberie dans une maison qui, avec le temps, après des corrections et des modifications, devient si capricieux et si couplé que lorsqu’il répare une chose, affecte et éventuellement casse une autre chose qui doit ensuite être réparée et que vous ne pouvez absolument pas savoir. tous les endroits où cela se produira.

C'est ma parole contre l'ancien consultant. Alors, comment puis-je donner des exemples simples et concrets qu'un client non technique comprendrait?

Vous avez raison, le mot est contre le visuel que le consultant précédent a créé dans leur tête. Ma suggestion est de faire exactement ce que vous demandez, de donner des exemples simples et concrets. Puisqu'il s'agit d'une refonte, montrez comment un fragment HTML défini dans le code compilé est affiché avec le reste d'une page HTML et en quoi cela affecte ou non le reste de la page. Peut-être que ce même code compilé restitue le balisage après l'application d'une règle "commerciale". Montre la différence.

C'est un problème difficile et TRÈS commun. Bonne chance.

Soyez honnête et soyez direct.

Mais surtout, ne prenez pas un travail qui ne répondra pas à vos attentes. La plupart des gens ne se rendent pas compte qu'un entrepreneur peut renvoyer un client, ils peuvent et devraient si le travail est plus pénible qu'il ne vaut la peine.

Voici une analogie que j'ai utilisée (bien que je ne garantisse pas son efficacité): Imaginez que leur site Web est une machine physique, comme une presse à imprimer mécanique qui accepte en quelque sorte les entrées.

Ils pensent probablement que la machine a un composant qui utilise X et un autre qui utilise Y. En réalité, il s’agit d’une vingtaine de machines similaires. Certains d'entre eux ne font plus rien, tous essayent de préformer des fonctions que les autres font déjà et personne d'autre que le précédent consultant n'a jamais rien vu de tel.

"Voir ce gizmo ici qui analyse les variables de publication et envoie ensuite ce composant dans un gouffre d'incroyables choix? désinfectez l'entrée et certains ne le font pas (ou tous ne le font pas) et sans lire tout le texte, je ne sais pas lequel. "

Un point qui n’a pas encore été mentionné est que vous pourriez simplement outrepasser ce que votre client veut vraiment de vous dans ce cas. Trop réussir est excellent et peut vous donner beaucoup de satisfaction au travail. Mais si le client ne s'en soucie tout simplement pas, pense que les performances actuelles sont "assez bonnes" et souhaite seulement quelques mises à jour mineures, il peut s'avérer impossible de le persuader de faire un investissement important pour réorganiser la base de code.

À ce stade, vous devrez probablement décider s’il faut s’appuyer sur des principes et refuser de prendre un travail qui vous obligerait à attacher votre bonne réputation à un gâchis de code embarrassant ou si vous pouvez vous tenir le nez, entrer, faire le travail. avec du ruban adhésif, et sortir avec votre paiement.

Si vous décidez toutefois d'aller de l'avant avec le travail sur ruban adhésif, veillez à documenter, documenter, documenter et être aussi transparent que possible. La dernière chose que vous voulez, c'est que vous soyez blâmé pour quelque chose qui ne va pas dans l'avenir et qui résulte d'une faille d'application que vous avez mise en garde, mais que ce dernier a décidé de ne pas avoir assez importante à gérer à l'époque.

En ce qui concerne les risques liés à l’injection SQL, comme d’autres l’ont déjà dit, vous devriez être en mesure de leur démontrer les dangers de cette manière de manière à montrer les risques, sans pour autant que la production soit réellement destructrice. Mais encore une fois, s’ils le voient et ne se soucient pas assez de vous payer pour le réparer, vous avez fait preuve de diligence de bonne foi dans cette affaire.

Noob Sauce ne doit pas participer à un projet et suggérer une réécriture, effectuer un petit sous-ensemble des modifications et utiliser celles-ci pour illustrer à quel point cela aurait pu être beaucoup plus simple et moins cher . Vous avez ensuite un argument probant pour expliquer pourquoi l'augmentation des coûts d'un développement plus propre entraînera une réduction des coûts de maintenance et un développement plus rapide à long terme, compte tenu du faible coût lié à la fonte.

N'oubliez jamais que, fondamentalement, vous leur demandez de vous payer pour vous simplifier la vie. Dans leur esprit, le simple besoin de trouver "le type" qui peut X fonctionnalités au coût de Y et une amplification de la complexité de votre projet peut simplement éliminer l'occasion. pour vous. La route est rude quand un mois après le début de la réécriture et que vous rencontrez le développeur d'origine pour vous rendre compte que l'application entière était écrite dans une fenêtre extrêmement contractée par un développeur qui comprenait parfaitement tous les compromis qui avaient été faits. Si l’application semble horrible à l’intérieur mais que l’extérieur fonctionne bien, comme vous le dites, il est fort probable que ce sera le cas. La dette technique dans une base de code est souvent le produit des contraintes de ressources dans lesquelles le code a été développé. S'ils ne constituent pas une équipe, ils sous-traitent plutôt des tâches ...

Je dis juste

Je vais me faire l'avocat du diable ici (un peu dans le sens de ce que @khrome dit: "vous ne payez pas les clients pour vous faciliter la vie"). J'irais même jusqu'à dire que le cas que vous avez présenté est trop unilatéral parce que vous l'avez décrit de manière générale. La plupart des consultants entrant dans un nouveau projet jetteraient un éclairage négatif sur le précédent.

Cela dit, je vais essayer de vous adresser les problèmes point par point:

• Injections SQL . OK, donc je suppose que le programmeur utilisait des concaténations de chaînes au lieu de requêtes paramétrées et / ou de procédures stockées. Ceci est très facile à résoudre, en particulier dans ADO.NET ... Personnellement, je le mentionnerais au client, mais je n'en ferais pas trop.
• HTML est généré dans la logique métier et serait un cauchemar à résoudre . OK, mec, c'est l'un de ceux où tu me donnes plus de détails. Sauf si vous utilisez MVC, c'est une tendance à se produire ... mais ce n'est pas nécessairement une mauvaise chose ... c'est l'une de ces choses où la plupart des programmeurs diraient " goto is bad; ne l'utilisez jamais" mais vous savez quoi? J'ai utilisé goto où cela avait du sens! Alors, êtes-vous sûr qu'ils n'utilisent pas de classes auxiliaires partageant le même espace de noms que la DLL de code métier? Encore une fois, ce n'est pas si difficile à isoler.
• la logique métier est répartie dans toute l'application, il y a beaucoup de duplication et un code sans issue qui ne fait rien. . Et? Le client vous demande simplement de changer HTML / CSS. Pourquoi vous soucier de ces problèmes du tout?
• il continue à générer des exceptions qui sont en train d'être étouffées, de sorte que le site semble fonctionner correctement . Encore une fois, très vague. Les exceptions sont normales dans toutes les applications, c'est pourquoi nous avons des clauses try / catch dans notre code. À moins qu'ils ne se retrouvent dans l'interface utilisateur et ruinent l'expérience utilisateur (comme l'affichage non nécessaire de HTTP 500), je ne pense pas que ce soit une chose à laquelle vous devriez vous intéresser, non plus.

Donc, en bref, je vous conseillerais de prendre la grande route. Si vous pensez que cela ne vaut pas la peine et que vous souhaitez le réécrire aux dépens de votre client, éloignez-vous de votre travail. Sérieusement, à la fin, le client paie votre temps pour faire fonctionner le tout avec un minimum de $ $$$.

Au cours de mes nombreuses années d'expérience dans le domaine, je dis toujours que les meilleurs programmeurs que j'ai rencontrés sont ceux qui peuvent rendre un système stable en écrivant le moins de code possible , et non en réécrivant tout .

Edit: Je vois déjà que ma réponse n'est pas la plus populaire (je m'y attendais déjà) mais je maintiens ma réponse. J'ai édité ceci pour le rendre moins sournois. ;-)

Certes, les attaques par injection SQL et d’autres failles fonctionnelles de l’application ont préséance, mais vous pouvez également "démontrer" une mauvaise qualité de code et de bonnes pratiques. Avec les outils de métrique du code, vous pouvez clairement montrer à quel point le code est mauvais et lui montrer à quel point cela va représenter un coût supplémentaire pour les modifications futures et la résolution des bugs. Je ne suis pas familier avec l'environnement .net, mais je suis sûr qu'il y en a plusieurs à prendre.