Wie kann ich während der Überprüfung taktvoll Verbesserungen des schlecht gestalteten Codes anderer vorschlagen?

130

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.

Yony
quelle
25
Stellen Sie zunächst sicher, dass Ihre Ansicht nicht subjektiv ist. Sehr oft schreiben Entwickler den Code anderer Leute ab, weil sie einfach einen anderen Stil oder Dialekt mögen. Wenn dies nicht der Fall ist, versuchen Sie, nacheinander Verbesserungen anzubieten.
Coder
Da eine Menge Software-Entwicklung mit Kompromissen zu tun hat und ich von Code-Kunstfertigkeit spreche, die hauptsächlich auf Design basiert, sind viele Kommentare zur Code-Überprüfung subjektiv. Deshalb habe ich gefragt : „Wie gehen Sie über was darauf hindeutet , Verbesserungen“. Es ist sicherlich nicht mein Ziel, etwas zu diktieren.
Yony
5
Die Frage klingt so, als ob die Codeüberprüfung nach Abschluss des Tests stattfindet. Wenn dies der Fall ist, verschwenden Sie entweder Zeit mit dem Testen (Änderungen müssen erneut getestet werden) oder erschweren das Durchführen von Änderungen aufgrund der Codeüberprüfung erheblich (warum funktioniert der Code bereits?).
Vaughandroid
3
@Baqueta - Warum Code überprüfen und die Zeit mehrerer Personen verschwenden, wenn Sie keine Ahnung haben, ob es noch funktioniert?
Dunk
4
@Baqueta Es gibt offensichtlich verschiedene Arten von Tests. Wenn sie nützlich sein sollen, sollten Codeüberprüfungen nach anfänglichen Tests wie Unit-Tests (so dass Sie wissen, dass es funktioniert) und vor abschließenden Tests wie Benutzerakzeptanztests (so dass Änderungen keine Bürokratie verursachen) durchgeführt werden. .
Caleb

Antworten:

160
  1. Ü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.

  2. 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.

  3. 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.

  4. 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.

  5. 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.

  6. 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.

Caleb
quelle
3
In der Tat kommt es darauf an, Ihre Anliegen zu "verkaufen". Wie bereits erwähnt: Nutzen und Mehrwert herausstellen. Das ist ein harter Job, auch nach meiner Erfahrung.
Wivani
4
Die eigene Motivation zu verstehen, heißt nicht nur zu verkaufen. Sie müssen verstehen, warum Sie einige Techniken mögen und andere nicht, damit Sie wissen, wann Ihre Faustregeln gültig sind und wann nicht. Viele, viele Probleme ergeben sich aus erfahrenen Programmierern, die korrekte Techniken in den falschen Situationen anwenden.
Jørgen Fogh,
1
Ihre Punkte lassen es scheinen, als wäre Golfen in
Ordnung
2
+1 für die gesamte Antwort, aber insbesondere für "Wenn der Code unordentlich ist, besteht eine
größere
2
Interessanterweise scheint die Folge von Punkt (6) zu sein, dass die Schnittstellenqualität wichtiger ist als die Implementierungsqualität
Brad Thomas,
16

Es gibt einen Sweet-Spot für die Wertschöpfung durch Refactoring. Änderungen müssen drei Dinge bewirken:

  • Verbessern Sie den Code, der sich wahrscheinlich ändern wird
  • Klarheit erhöhen
  • kosten am wenigsten aufwand

Überlegungen:

  1. Wir wissen, dass sauberer Code kostengünstiger zu schreiben und zu warten ist und mehr Spaß macht, daran zu arbeiten. Ihre Aufgabe ist es, diese Idee an die Menschen in Ihrem Unternehmen zu verkaufen. Denken Sie wie ein Verkäufer, nicht wie ein arroganter Nörgler (dh nicht wie ich).
  2. Sie können nicht gewinnen, Sie können nur weniger verlieren.
  3. Konzentrieren Sie sich darauf, einen echten Mehrwert zu schaffen - nicht nur Schönheit. Ich mag es, wenn mein Code gut aussieht, muss aber manchmal akzeptieren, dass es mehr auf den Preis ankommt.
  4. Ein guter Weg, um den Sweet Spot zu finden, ist das Befolgen des Boy Scout-Prinzips. Wenn Sie an einem Codebereich arbeiten, lassen Sie ihn immer in einem besseren Zustand, als Sie ihn vorgefunden haben.
  5. Eine kleine Verbesserung ist besser als keine Verbesserung.
  6. Nutzen Sie automatisierte Werkzeuge. Zum Beispiel Werkzeuge , die nur ein wenig Formatierung aufzuräumen können eine machen Welt des Unterschiedes.
  7. Verkaufen Sie andere Ideen, die im Übrigen die Code-Klarheit verbessern. Zum Beispiel wird durch Unit-Tests die Zerlegung großer Methoden in kleinere gefördert.
Kramii
quelle
5
+1 für die Verwendung von automatisierten Werkzeugen. Schockierenderweise scheint es vielen Läden nicht wichtig zu sein, wie das Toolkit für Entwickler aussieht. Nur weil Sie über eine Quellcodeverwaltung, einen Editor und einen Compiler verfügen, wird Ihr Toolkit nicht vollständig.
Spencer Rathbun
4
@Spencer: Ich könnte nicht mehr zustimmen. Gleichzeitig bin ich frustriert von Entwicklern, die die Tools, die sie haben, nicht verwenden - weil sie die Funktion oder die Vorteile nicht kennen oder einfach nur faul sind. Die meisten modernen IDEs verfügen über eine integrierte Code-Formatierungsfunktion, die nur ein paar Tastatureingaben erfordert. Einige Entwickler verwenden sie jedoch nicht.
Kramii
2
Wahr. Aber ich schließe das unter dem Laden selbst nicht ein. Ihre Entwickler kennen möglicherweise einige Funktionen im aktuellen Toolset nicht, insbesondere wenn das Management sich nie die Mühe gemacht hat, Standards zu erstellen. Zweitens sind viele IDEs sehr groß und verfügen über umfangreiche Funktionen. Ich benutze vim seit ein paar Jahren und weiß immer noch nicht, was ich alles damit anfangen kann. Wenn Sie mich in Visual Studio fallen ließen, würde ich 90% der Funktionalität unberührt lassen, bis ich Zeit hatte, sie zu durchforsten. Dann kann ich mich vielleicht nicht daran erinnern.
Spencer Rathbun
14

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.

hotpaw2
quelle
Wenn Sie dort gelandet sind, haben die Prozesse, die zu diesem Punkt geführt haben, Sie wahrscheinlich gescheitert.
Tim Abell
9

Die Codeüberprüfung dient drei Zwecken:

  1. Nach Fehlern suchen

  2. Überprüfen, ob der Code verbessert werden kann

  3. 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 $Bnun auswerten - oder besser gesagt. Lassen Sie Ihren Teamleiter / Manager $Avs bewerten $Bund 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:

  • SEHR klar abgrenzen "muss beheben" Fehler / Probleme von "Dies ist eine bewährte Methode und sollte wirklich behoben werden, WENN wir Ressourcen sparen können - siehe beigefügte Pro / Con-Analyse" Designverbesserungen (das für # 2 oben beschriebene Zeug beifügen) vs " Dies sind allgemeine Richtlinien, von denen ich denke, dass sie Ihnen dabei helfen würden, die Robustheit Ihres Codes zu verbessern, damit Sie die optionalen Änderungen des Codes einfacher beibehalten können. Bitte beachten Sie die Formulierung - es geht nicht darum, "Ihren Code so zu machen, wie ich es will" - es geht darum, "wenn Sie dies tun, erhalten Sie die Vorteile a, b, c". Der Ton und die Herangehensweise sind wichtig.
DVK
quelle
2
Auf # 3 dienen Code-Reviews nicht nur dazu, den Autor des Codes zu unterrichten. Ein Review kann eine gute Möglichkeit für weniger erfahrene Entwickler sein, und für erfahrene Programmierer, die neu im Team sind, um sich über Codierungsstandards zu informieren. Das Besprechen von Problemen in einer Gruppe kann auch zu Erkenntnissen über das Produkt führen.
Caleb
1
@ Caleb - guter Punkt. Ich wollte nicht zu viele Punkte machen, also wurde dieser aus der Gliederung heraus bearbeitet, aber es ist immer noch ein gültiger Punkt.
DVK
# 4 Cross-Training-Entwickler über neue Funktionen
Juan Mendes
1
# 5 - Der Hauptzweck der
Codeüberprüfung
8

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.

Vielen Dank an Papathanasiou
quelle
5
Sind Termine nicht immer am Horizont?
FreeAsInBeer
8

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. :-)

Roger CS Wernersson
quelle
5

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.

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:

Hey Boss, wir waren im Zeitplan und hatten alles am Laufen, aber jetzt werden wir eine Menge Sachen umbauen, damit wir in Zukunft Feature X hinzufügen können.

oder

Hey Boss, wir sind bereit, loszulassen. Wenn wir jemals Feature X hinzufügen müssen, kann dies einige zusätzliche Tage in Anspruch nehmen.

Wenn Sie eines von beiden sagten, würde Ihr Chef wahrscheinlich sagen:

Wer hat etwas über Feature X gesagt?

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.

Morgan Herlocker
quelle
Auch bekannt als YAGNI c2.com/cgi/wiki?YouArentGonnaNeedIt
Juan Mendes
Wie wäre es mit: "Hey Boss, Sie kennen das gewünschte Feature X, nun, wir brauchen ein paar Tage, bevor wir anfangen können, daran zu arbeiten." Das würde ihm auch gefallen. YAGNI ist keine Entschuldigung, um unordentlichen Code zu erstellen oder Code unordentlich zu hinterlassen. Eine kleine technische Verschuldung ist kein großes Problem, aber die Schulden müssen zurückgezahlt werden. Je früher Sie sie zurückzahlen, desto billiger wird sie.
Thorsal
5

[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 .

Bildbeschreibung hier eingeben WTFs / Minute

Ellen Spertus
quelle
4

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:

Ich frage mich, ob es besser wäre, ...

oder

Ist es sinnvoll, ...

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!

Ed Staub
quelle
1
Es ist selten, dass ich sagen würde "Ich frage mich, ob es besser wäre, ...". Ich würde nur sagen, wenn ich mir nicht sicher wäre - in diesem Fall kann der Autor frei überlegen, ob es besser wäre oder nicht, und er kann es frei ändern oder nicht. Normalerweise sage ich "Ich würde X machen". (Manchmal sage ich "Ich hätte X gemacht, aber Ihr Ansatz ist besser"). Was bedeutet, ich denke X ist besser, aber Sie können nicht zustimmen. Oder ich sage "das funktioniert nicht" oder "das ist gefährlich" und Sie ändern es besser. Manchmal wird mir gesagt, "das funktioniert nicht", und normalerweise schaue ich mir den Code an, dann sage ich "Oh shit", und dann repariere ich es.
gnasher729
3

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.

Tom Clarkson
quelle
3

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:

  1. Sie müssen Ihren eigenen persönlichen "Einfluss" (eine Form der "Macht", die indirekt ist) und / oder Ihre Fähigkeit, "zu überzeugen", einsetzen
  2. Beteiligen Sie sich an der Gruppe "Code-Prozess" Ihres Unternehmens und beginnen Sie, der "Code-Pflege" eine höhere Priorität zuzuweisen.
  3. Beißen Sie in die Kugel und lernen Sie, wie man beschissenen Code schneller / flüssiger liest, damit Sie nicht auf dem beschissenen Code hängen bleiben (es hört sich so an, als würden Sie immer wieder hängen bleiben oder langsamer werden, wenn Sie auf beschissenen Code stoßen).
    • Dies macht Sie auch zu einem stärkeren Programmierer.
    • Und damit können Sie den beschissenen Code korrigieren, wenn Sie an beschissenem Code arbeiten.
    • Auf diese Weise können Sie auch an mehr Projekten arbeiten, da viele Projekte einen beschissenen Code haben, der funktionsfähig ist ... aber viel beschissenen Code.
  4. Mit gutem Beispiel vorangehen. Machen Sie Ihren Code besser ... aber versuchen Sie nicht, ein Perfektionist zu sein.
    • Denn dann wirst du "der Langsame, der keine Termine einhält, immer kritisiert und denkt, er sei besser als alle anderen".

Ich finde, es gibt keine Silberkugel. Sie müssen alle drei verwenden und Sie müssen kreativ sein, wenn Sie alle drei verwenden.

Trevor Boyd Smith
quelle
Ich wünschte, ich könnte lernen, # 3, ich bin so frustriert mit schlechtem Code, dass ich es schwer habe, es zu verstehen ... ständig daran zu arbeiten ....
Juan Mendes
Design ist fehlerhaft? Welches Design?
gnasher729
1

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.

entwurde
quelle
1

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

  1. es wird optimal sein, wenn wir es so machen.
  2. Wenn Sie es auf diese Weise schreiben, wird es schneller ausgeführt.
  3. Ihr Code ist viel besser lesbar, wenn Sie "dies", "dies" und "dies" tun.

Solche Kommentare werden ernst genommen, auch wenn Ihre Fristen in Kürze eintreffen. Und wird voraussichtlich im nächsten Entwicklungszyklus implementiert.

Jay D
quelle
0

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.

kgambrah
quelle
0

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:

  • Sind die vorgeschlagenen Änderungen wirklich eine Verbesserung gegenüber dem, was vorhanden ist?
  • Oder nur eine Ansichtssache?
  • Hat der Rezensent richtig erklärt, warum, es sei denn, etwas ist sehr offensichtlich ?
  • Wie lange hat es gedauert? (Ich habe monatelange Überprüfungen gesehen, bei denen der Entwickler die Verantwortung für die Lösung aller zahlreichen Zusammenführungskonflikte trägt.)
  • Ton?

Es ist viel einfacher, eine gute Codeüberprüfung zu akzeptieren, als einige meist fragwürdige Ego-Pflege.

h22
quelle
0

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:

  • Haben Sie ein gemeinsames Verständnis über Kodierungsstandards und Designprinzipien.
  • Kritisieren oder überprüfen Sie niemals den Entwickler , sondern nur den Code . Vermeiden Sie das Wort "Sie" oder "Ihr Code". Sprechen Sie einfach über den zu überprüfenden Code, unabhängig von einem Entwickler.
  • Setzen Sie Ihren Stolz auf die Qualität des vervollständigten Codes, sodass Kommentare, die zur Verbesserung des Endergebnisses beitragen, geschätzt werden.
  • Vorschlagen Verbesserungen eher als die Nachfrage. Immer einen Dialog haben, wenn Sie nicht einverstanden sind. Versuchen Sie, den anderen Standpunkt zu verstehen, wenn Sie anderer Meinung sind.
  • Lassen Sie die Bewertungen in beide Richtungen gehen. Das heißt, die Person, die Sie überprüft haben, überprüft Ihren Code als Nächstes. Keine "One-Way" -Bewertungen vorhanden.

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!

JacquesB
quelle