Ich bin ein großer Anhänger von sauberem Code und Code-Handwerkskunst, obwohl ich derzeit an einem Job bin, bei dem dies nicht als oberste Priorität angesehen wird. Ich finde mich manchmal in einer Situation wieder, in der der Code eines Kollegen mit unordentlichem Design und sehr wenig Sorge um zukünftige Wartungsarbeiten durchsetzt ist, obwohl er funktionsfähig ist und kaum bis gar keine Fehler enthält.
Wie gehen Sie vor, um Verbesserungen in einer Codeüberprüfung vorzuschlagen, wenn Sie der Meinung sind, dass so viel geändert werden muss und eine Frist ansteht? Denken Sie daran, dass die Vorschläge für Verbesserungen, die nach Ablauf der Frist vorgenommen werden, dazu führen können, dass sie aufgrund neuer Funktionen und Fehlerbehebungen ihre Priorität verlieren.
Antworten:
Überprüfen Sie Ihre Motivation. Wenn Sie der Meinung sind, dass der Code geändert werden sollte, sollten Sie in der Lage sein, einen Grund anzugeben , warum Sie der Meinung sind, dass er geändert werden sollte. Und dieser Grund sollte konkreter sein als "Ich hätte es anders gemacht" oder "es ist hässlich". Wenn Sie nicht auf einen Nutzen hinweisen können, der sich aus Ihrer vorgeschlagenen Änderung ergibt, ist es nicht sinnvoll, Zeit (auch bekannt als Geld) für eine Änderung aufzuwenden.
Jede Codezeile im Projekt ist eine Zeile, die gepflegt werden muss. Code sollte so lang sein, wie es sein muss, um die Arbeit zu erledigen und leicht zu verstehen, und nicht länger. Wenn Sie den Code verkürzen können, ohne an Klarheit zu verlieren, ist das gut. Wenn Sie es schaffen, während Sie die Übersichtlichkeit erhöhen, ist das viel besser.
Code ist wie Beton: Es ist schwieriger, ihn zu ändern, nachdem er eine Weile gesessen hat. Schlagen Sie Ihre Änderungen möglichst frühzeitig vor, damit Kosten und Risiko von Änderungen minimiert werden.
Jede Änderung kostet Geld. Das Umschreiben von Code, der funktioniert und wahrscheinlich nicht geändert werden muss, kann unnötig sein. Konzentrieren Sie sich auf die Abschnitte, die Änderungen unterliegen oder die für das Projekt am wichtigsten sind.
Form folgt Funktion und manchmal umgekehrt. Wenn der Code unordentlich ist, besteht eine höhere Wahrscheinlichkeit, dass er auch Fehler enthält. Suchen Sie nach diesen Fehlern und kritisieren Sie die fehlerhafte Funktionalität und nicht den ästhetischen Reiz des Codes. Schlagen Sie Verbesserungen vor, damit der Code besser funktioniert und die Funktionsweise des Codes leichter überprüft werden kann.
Unterscheiden Sie zwischen Design und Implementierung. Eine wichtige Klasse mit einer beschissenen Schnittstelle kann sich durch ein Projekt wie Krebs verbreiten. Dies verringert nicht nur die Qualität des restlichen Projekts, sondern erhöht auch die Schwierigkeit, den Schaden zu beheben. Andererseits sollte eine Klasse mit einer gut gestalteten Oberfläche, aber einer miesen Implementierung keine große Sache sein. Sie können die Klasse jederzeit erneut implementieren, um eine bessere Leistung oder Zuverlässigkeit zu erzielen. Wenn es richtig funktioniert und schnell genug ist, können Sie es in Ruhe lassen und sich sicher fühlen, dass seine Kruft gut eingekapselt ist.
Um alle oben genannten Punkte zusammenzufassen: Stellen Sie sicher, dass Ihre vorgeschlagenen Änderungen einen Mehrwert schaffen.
quelle
Es gibt einen Sweet-Spot für die Wertschöpfung durch Refactoring. Änderungen müssen drei Dinge bewirken:
Überlegungen:
quelle
Wenn der Code ohne schwerwiegende Fehler funktioniert und eine größere Frist (wie in Bezug auf Gewinn- und Verlustrechnung oder Unternehmens-PR) unmittelbar bevorsteht, ist es zu spät, Verbesserungen vorzuschlagen, die größere Änderungen erfordern. Selbst Verbesserungen am Code können Risiken für die Projektbereitstellung mit sich bringen. Die Zeit für Verbesserungen war früher im Projekt, als mehr Zeit vorhanden war, um in die zukünftige Robustheit der Codebasis zu investieren.
quelle
Die Codeüberprüfung dient drei Zwecken:
Nach Fehlern suchen
Überprüfen, ob der Code verbessert werden kann
Lehrmittel für jeden, der den Code geschrieben hat.
Die Bewertung des Designs / der Codequalität erfolgt natürlich über die Punkte 2 und 3.
So weit wie # 2:
Stellen Sie SEHR klar, welche Vorteile sich aus den vorgeschlagenen Änderungen und den zu reparierenden Kosten ergeben. Bei jeder Geschäftsentscheidung sollte es sich um eine Kosten-Nutzen-Analyse handeln.
Beispiel: "Die X-Methode für das Design würde die Wahrscheinlichkeit des Auftretens von Fehler Y beim Ändern von Z erheblich verringern, und wir wissen, dass dieser Codeteil alle 2 Wochen Änderungen des Typs Z unterzogen wird. Die Kosten für die Behandlung von Produktionsausfällen aufgrund von Fehler Y + Auffinden des Fehlers + Fixieren und das Update + Opportunitätskosten der Freigabe von nicht den nächsten Satz von featires liefern ist
$A
, während die Kosten für die Reinigung des Code jetzt und Opportunitätskosten (zB Preis für den Versand zu spät oder mit weniger Funktionen) nach oben ist$B
nun auswerten - oder besser gesagt. Lassen Sie Ihren Teamleiter / Manager$A
vs bewerten$B
und entscheiden.Dies wird dem intelligenten Team helfen, dies effektiv zu verwalten. ZB treffen sie eine rationale Entscheidung unter Verwendung der VOLLSTÄNDIGEN Informationen
Dies wird (besonders wenn Sie dies gut ausdrücken) IHREN Status erhöhen - z. B. sind Sie jemand, der intelligent genug ist, um die Vorteile eines besseren Designs zu erkennen, UND intelligent genug, um es nicht religiös zu fordern, ohne geschäftliche Erwägungen abzuwägen.
UND in dem wahrscheinlichen Fall, dass Fehler Z auftritt, gewinnen Sie bei den nächsten Vorschlägen noch viel mehr Einfluss.
So weit wie # 3:
quelle
Wählen Sie Ihre Schlachten, wenn eine Frist bevorsteht, dann tun Sie nichts. Wenn das nächste Mal jemand anderes Code überprüft oder wartet und weiterhin Probleme damit hat, wenden Sie sich an ihn mit der Idee, dass Sie sich als Team bei der Überprüfung des Codes stärker auf die Codequalität konzentrieren sollten, damit Sie später nicht so viele Probleme haben.
Sie sollten den Wert sehen, bevor sie die zusätzliche Arbeit erledigen.
quelle
Ich beginne meinen Kommentar immer mit "Ich würde", was signalisiert, dass meine einfach eine von vielen Ansichten ist.
Ich gebe auch immer einen Grund an.
„ Ich würde diesen Block in ein Verfahren extrahieren , weil der Lesbarkeit.“
Ich kommentiere alles; groß und Klein. Manchmal mache ich mehr als hundert Kommentare zu einer Änderung. In diesem Fall empfehle ich auch die Paarprogrammierung und biete mich als Wing-Man an.
Ich versuche aus Gründen, eine gemeinsame Sprache zu etablieren. Lesbarkeit, DRY, SRP usw.
Ich habe auch eine Präsentation zu Clean Code und Refactoring erstellt, in der erklärt wird, warum und wie, was ich meinen Kollegen vorgehalten habe. Ich habe es bisher dreimal gehalten, und ein Beratungsunternehmen, das wir in Anspruch nehmen, hat mich gebeten, es erneut für sie zu halten.
Aber manche hören trotzdem nicht zu. Dann bin ich am Zug. Ich bin der Designleiter. Die Codequalität liegt in meiner Verantwortung. Diese Änderung wird auf meiner Uhr in ihrem aktuellen Zustand nicht durchkommen.
Bitte beachten Sie, dass ich mehr als bereit bin, meine Kommentare zu kommentieren. Aus technischen Gründen, Fristen, Prototypen usw. muss ich noch viel über das Codieren lernen und werde immer auf die Vernunft hören.
Oh, und ich habe kürzlich angeboten, dem ersten Mitglied meines Teams ein Mittagessen zu kaufen, das eine nicht triviale Änderung eingereicht hat, zu der ich keinen Kommentar abgegeben habe. (Hey, du musst auch Spaß haben. :-)
quelle
Dieser Code ist fertig. Ab einem bestimmten Punkt sind Neugestaltungen zu kostspielig, um sie zu rechtfertigen. Wenn der Code bereits ohne Bugs funktioniert, ist dies ein unmöglicher Verkauf. Schlagen Sie einige Möglichkeiten vor, um dies in Zukunft zu bereinigen und fortzufahren. Wenn der Code in der Zukunft kaputt geht, überprüfen Sie den Wert eines Redesigns. Es darf nie kaputt gehen, das wäre toll. So oder so, Sie sind an dem Punkt angelangt, an dem es Sinn macht zu spielen, dass es nicht kaputt geht, da die Kosten jetzt oder später gleich sind: langwieriges, schreckliches Redesign.
Was Sie in Zukunft tun müssen, ist engere Entwicklungsiterationen. Wenn Sie in der Lage gewesen wären, diesen Code zu überprüfen, bevor alle Arbeiten zum Ausbügeln von Fehlern durchgeführt wurden, hätte es Sinn gemacht, eine Neugestaltung vorzuschlagen. Gegen Ende ist es nie sinnvoll, größere Umgestaltungen vorzunehmen, es sei denn, der Code ist auf eine grundsätzlich nicht zu wartende Weise geschrieben und Sie wissen mit Sicherheit, dass der Code bald nach der Veröffentlichung geändert werden muss.
Überlegen Sie sich bei der Wahl zwischen den beiden Optionen (Refaktor oder kein Refaktor), was sich nach einem intelligenteren Verkauf anhört:
oder
Wenn Sie eines von beiden sagten, würde Ihr Chef wahrscheinlich sagen:
Das Fazit ist, dass manchmal ein bisschen technische Verschuldung Sinn macht, wenn Sie bestimmte Fehler nicht korrigieren konnten, als es billig war (frühe Iterationen). Das Entwerfen von Qualitätscodes hat eine abnehmende Rendite, wenn Sie sich einem abgeschlossenen Feature und der Frist nähern.
quelle
[Diese Antwort ist etwas breiter als die ursprünglich gestellte Frage, da dies die Weiterleitung für so viele andere Fragen zur Codeüberprüfung ist.]
Hier sind einige Prinzipien, die ich nützlich finde:
Privat kritisieren, öffentlich loben. Informieren Sie jemanden über einen Fehler in seinem Code. Wenn sie etwas Brillantes taten oder eine Aufgabe übernahmen, die niemand wollte, loben Sie sie bei einem Gruppentreffen oder in einer E-Mail an das Team.
Teile deine eigenen Fehler. Ich teile die Geschichte meiner katastrophalen ersten Codeüberprüfung (erhalten) mit Studenten und Nachwuchskollegen. Außerdem habe ich den Schülern mitgeteilt, dass ich ihren Fehler so schnell erkannt habe, weil ich es vor mir selbst geschafft habe. In einer Codeüberprüfung könnte dies folgendermaßen lauten: "Ich glaube, Sie haben die Indexvariablen hier falsch angegeben. Ich überprüfe dies immer, da ich meine Indizes falsch angegeben und ein Rechenzentrum heruntergefahren habe." [Ja, das ist eine wahre Geschichte.]
Denken Sie daran, positive Kommentare abzugeben. Ein kurzes "schön!" oder "ordentlicher Trick!" kann den Tag eines jungen oder unsicheren Programmierers machen.
Angenommen, die andere Person ist intelligent, aber manchmal nachlässig. Sagen Sie nicht: "Wie erwarten Sie, dass der Aufrufer den Rückgabewert erhält, wenn Sie ihn nicht tatsächlich zurückgeben ?!" Sprich: "Sieht so aus, als hätten Sie die Rückgabeerklärung vergessen." Denken Sie daran, dass Sie in Ihren frühen Tagen schrecklichen Code geschrieben haben. Wie jemand einmal sagte: "Wenn Sie sich nicht für Ihren Code vor einem Jahr schämen, lernen Sie nicht."
Bewahre den Sarkasmus / die Lächerlichkeit für Freunde auf, die nicht an deinem Arbeitsplatz sind. Wenn der Code episch schrecklich ist, scherzen Sie woanders darüber. (Ich finde es bequem, mit einem anderen Programmierer verheiratet zu sein.) Zum Beispiel würde ich die folgenden Cartoons (oder diesen ) nicht mit meinen Kollegen teilen .
quelle
Wenn ein Löffel Zucker hilft, die Medizin zu senken, und das, was falsch ist, kurz und bündig ausgedrückt werden kann - es sind keine 20 Dinge falsch -, führe ich eine Form an, die besagt, dass ich keinen Einsatz habe und kein Ego in das investiert habe, was ich will gehört werden. Normalerweise ist es so etwas wie:
oder
Wenn die Gründe ziemlich offensichtlich sind, gebe ich sie nicht an. Dies gibt anderen Leuten die Möglichkeit, eine intellektuelle Verantwortung für den Vorschlag zu übernehmen, wie in:
"Ja, das ist eine gute Idee, denn < Ihr offensichtlicher Grund hier >."
Wenn die Verbesserung ziemlich offensichtlich ist, aber mich nicht zum Idioten macht, weil ich nicht daran denke, und der Grund dafür einen mit dem Hörer geteilten Wert widerspiegelt, dann schlage ich es manchmal nicht einmal vor, stattdessen:
Ich frage mich, ob es einen Weg gibt ...
Dies ist nur für den Umgang mit wirklich empfindlichen Leuten gedacht - bei den meisten meiner Kollegen lasse ich sie einfach haben!
quelle
Code Reviews zielen nicht immer darauf ab, Verbesserungen vorzunehmen.
Eine Überprüfung gegen Ende eines Projekts, wie es hier der Fall zu sein scheint, dient nur dazu, dass jeder weiß, wo er suchen soll, wenn Fehler auftreten (oder für ein besser gestaltetes Projekt, was möglicherweise für eine spätere Wiederverwendung verfügbar ist). Was auch immer das Ergebnis der Überprüfung sein mag, es ist einfach keine Zeit, etwas zu ändern.
Um tatsächlich Änderungen vorzunehmen, müssen Sie den Code und das Design viel früher im Projekt besprechen - Code ist viel einfacher zu ändern, wenn er nur als Diskussion über mögliche Ansätze existiert.
quelle
Ihre Frage lautet "Wie überprüfe ich schlecht gestalteten Code?":
Die Antwort IMO ist einfach. Sprechen Sie über das DESIGN des Codes und wie das Design fehlerhaft ist oder nicht den Anforderungen entspricht. Wenn Sie auf ein fehlerhaftes Design hinweisen oder "die Anforderungen nicht erfüllen", muss der Entwickler seinen Code ändern, da er nicht das tut, was er tun muss.
Wenn der Code "funktional ausreichend" und / oder "erfüllt Spezifikation" und / oder "erfüllt Anforderungen" ist:
Wenn Sie mit diesem Entwickler vergleichbar sind, verfügen Sie über keine direkte Berechtigung, mit der Sie ihm "mitteilen" könnten, dass Änderungen vorgenommen werden sollen.
Ihnen stehen noch einige Optionen zur Verfügung:
Ich finde, es gibt keine Silberkugel. Sie müssen alle drei verwenden und Sie müssen kreativ sein, wenn Sie alle drei verwenden.
quelle
Im Falle eines lähmenden schlechten Designs sollte Ihr Fokus auf der Maximierung der Kapselung liegen . Auf diese Weise wird es einfacher, die einzelnen Klassen / Dateien / Unterprogramme durch besser gestaltete Klassen zu ersetzen.
Achten Sie darauf, dass die öffentlichen Schnittstellen der Komponenten gut gestaltet sind und die internen Abläufe sorgfältig verborgen werden. Auch Datenspeicher-Wrapper sind unerlässlich. (Große Mengen gespeicherter Daten können sehr schwer zu ändern sein. Wenn Sie also "Implementierungsfehler" in andere Bereiche des Systems bekommen, sind Sie in Schwierigkeiten.)
Wenn Sie die Barrieren zwischen den Komponenten überwunden haben, konzentrieren Sie sich auf die Komponenten, die mit größter Wahrscheinlichkeit zu größeren Problemen führen.
Wiederholen Sie dies, bis die Frist abgelaufen ist oder bis das System "perfekt" ist.
quelle
Anstatt direkten Kritik am Code eines anderen zu üben, ist es immer besser, in unseren Kommentaren während der Codeüberprüfung eine konstruktive Haltung einzunehmen.
Ein Weg, dem ich folge, ist
Solche Kommentare werden ernst genommen, auch wenn Ihre Fristen in Kürze eintreffen. Und wird voraussichtlich im nächsten Entwicklungszyklus implementiert.
quelle
Die Codeüberprüfung muss in den Kultur- und Entwicklungszyklus integriert werden, um zu funktionieren. Es ist unwahrscheinlich, dass die Planung einer umfangreichen Codeüberprüfung am Ende der Entwicklung von Feature X funktioniert. Zuallererst wird es schwieriger, die Änderungen vorzunehmen, und es ist wahrscheinlich, dass sich jemand schämt - was zu einem Widerstand gegen die Aktivität führt.
Sie sollten frühzeitige und häufige Commits haben, verbunden mit Überprüfungen auf Commit-Ebene. Mit den vorhandenen Code-Analyse-Tools sind die meisten Überprüfungen schnell. Mithilfe automatisierter Tools zur Code-Analyse / -Überprüfung wie FindBugs und PMD können Sie eine Vielzahl von Designfehlern erkennen. Sie werden Ihnen jedoch nicht dabei helfen, Architekturprobleme zu lösen. Sie müssen also ein solides Design haben und das Gesamtsystem anhand dieses Designs beurteilen.
quelle
Erhöhen Sie die Qualität der Codeüberprüfungen.
Abgesehen von der Qualität des untersuchten Codes gibt es eine Qualität der Codeüberprüfung selbst:
Es ist viel einfacher, eine gute Codeüberprüfung zu akzeptieren, als einige meist fragwürdige Ego-Pflege.
quelle
Es gibt zwei bemerkenswerte Punkte in der Frage, den taktvollen Teil und die bevorstehende Frist . Dies sind unterschiedliche Themen - das erste ist eine Frage der Kommunikation und der Teamdynamik, das zweite eine Frage der Planung und Priorisierung.
Taktvoll . Ich gehe davon aus, dass Sie gebürstete Egos und negative Rückschläge gegen die Bewertungen vermeiden möchten. Einige Vorschläge:
Der zweite Teil ist die Priorisierung . Sie haben viele Verbesserungsvorschläge, aber da der Abgabetermin näher rückt, ist nur noch Zeit, einige davon zu übernehmen.
Nun, zuerst möchten Sie verhindern, dass dies überhaupt passiert! Hierzu führen Sie kontinuierliche, inkrementelle Überprüfungen durch. Lassen Sie einen Entwickler nicht wochenlang an einem Feature arbeiten und überprüfen Sie es im letzten Moment. Zweitens sollten Codeüberprüfungen und Zeit für die Implementierung von Überprüfungsvorschlägen Teil der regelmäßigen Planung und Schätzung für jede Aufgabe sein. Wenn nicht genügend Zeit für eine ordnungsgemäße Überprüfung vorhanden ist, ist bei der Planung ein Fehler aufgetreten.
Nehmen wir jedoch an, dass dabei ein Fehler aufgetreten ist und Sie nun mit einer Reihe von Überprüfungskommentaren konfrontiert sind, für deren Implementierung Sie keine Zeit haben. Sie müssen Prioritäten setzen. Nehmen Sie dann die Änderungen vor, die später am schwierigsten und riskantesten sind, wenn Sie sie verschieben.
Die Benennung von Bezeichnern im Quellcode ist für die Lesbarkeit und Wartbarkeit unglaublich wichtig, aber es ist auch ziemlich einfach und risikoarm, sie in Zukunft zu ändern. Gleiches gilt für die Code-Formatierung. Konzentriere dich also nicht auf das Zeug. Andererseits sollte die Vernunft öffentlich zugänglicher Schnittstellen höchste Priorität haben, da sie in Zukunft nur schwer zu ändern sind. Persistente Daten können nur schwer geändert werden - wenn Sie zum ersten Mal inkonsistente oder unvollständige Daten in einer Datenbank speichern, ist es wirklich schwierig, sie in Zukunft zu korrigieren.
Bereiche, die Gegenstand von Unit-Tests sind, weisen ein geringes Risiko auf. Sie können diese später jederzeit beheben. Bereiche, in denen dies nicht der Fall ist, die jedoch einer Prüfung unterzogen werden könnten , weisen ein geringeres Risiko auf als Bereiche, in denen keine Prüfung durchgeführt werden kann.
Angenommen, Sie haben eine große Menge Code ohne Komponententests und alle Arten von Problemen mit der Codequalität, einschließlich einer fest codierten Abhängigkeit von einem externen Dienst. Indem Sie diese Abhängigkeit einbauen, können Sie den Code-Chunk testbar machen. Dies bedeutet, dass Sie in Zukunft Tests hinzufügen und dann an der Behebung der restlichen Probleme arbeiten können. Mit der fest codierten Abhängigkeit können Sie nicht einmal Tests hinzufügen. Nehmen Sie also zuerst diesen Fix.
Aber bitte versuchen Sie zunächst zu vermeiden, dass Sie in dieses Szenario geraten!
quelle