Vereinbarkeit von Pfadfinderregel und opportunistischem Refactoring mit Codeüberprüfungen

55

Ich glaube fest an die Pfadfinderregel :

Überprüfen Sie ein Modul immer sauberer, als Sie es ausgecheckt haben. "Egal, wer der ursprüngliche Autor war, was wäre, wenn wir uns immer bemühen, das Modul zu verbessern, egal wie klein es ist. Was wäre das Ergebnis? Ich denke, wenn wir Alle folgten dieser einfachen Regel, wir würden das Ende der unaufhaltsamen Verschlechterung unserer Softwaresysteme sehen, stattdessen würden unsere Systeme im Laufe der Entwicklung immer besser werden und wir würden eher sehen, dass sich Teams um das System als Ganzes kümmern als nur Einzelpersonen, die sich um ihren eigenen kleinen Teil kümmern.

Ich bin auch ein großer Anhänger der verwandten Idee des Opportunistischen Refactorings :

Obwohl es Orte für geplante Umgestaltungsbemühungen gibt, ziehe ich es vor, das Umgestalten als opportunistische Aktivität zu fördern, die immer und überall durchgeführt wird, wo Code bereinigt werden muss - von wem auch immer. Dies bedeutet, dass jemand zu jeder Zeit einen Code sieht, der nicht so klar ist, wie er sein sollte. Er sollte die Gelegenheit nutzen, ihn genau dort zu beheben - oder zumindest innerhalb weniger Minuten

Beachten Sie insbesondere den folgenden Auszug aus dem Refactoring-Artikel:

Ich bin vorsichtig mit Entwicklungspraktiken, die Reibungen für opportunistisches Refactoring verursachen. Meiner Meinung nach tun die meisten Teams nicht genug Refactoring. Daher ist es wichtig, auf alles zu achten, was die Leute davon abhält, es zu tun. Um dies zu vermeiden, sollten Sie sich darüber im Klaren sein, wann immer Sie sich von einem kleinen Refactoring entmutigt fühlen. Eines, von dem Sie sicher sind, dass es nur ein oder zwei Minuten dauert. Jede solche Barriere ist ein Geruch, der eine Unterhaltung auslösen sollte. Machen Sie sich also eine Notiz über die Entmutigung und bringen Sie sie mit dem Team in Verbindung. Zumindest sollte dies bei Ihrer nächsten Retrospektive besprochen werden.

Wo ich arbeite, gibt es eine Entwicklungspraxis, die starke Reibung verursacht - Code Review (CR). Immer wenn ich etwas ändere, das nicht im Rahmen meiner "Aufgabe" liegt, werde ich von meinen Gutachtern zurechtgewiesen, dass ich die Änderung schwieriger zu überprüfen mache. Dies gilt insbesondere dann, wenn es sich um ein Refactoring handelt, da dies den Vergleich von "zeilenweisen" Unterschieden erschwert. Dieser Ansatz ist hier der Standard, was bedeutet, dass opportunistisches Refactoring selten durchgeführt wird und nur "geplantes" Refactoring (das in der Regel zu wenig, zu spät ist) stattfindet, wenn überhaupt.

Ich behaupte, dass sich die Vorteile lohnen und dass drei Rezensenten etwas härter arbeiten werden (um den Code vorher und nachher tatsächlich zu verstehen, anstatt sich den engen Umfang der geänderten Zeilen anzusehen - die Rezension selbst wäre allein schon deswegen besser ), damit die nächsten 100 Entwickler, die den Code lesen und warten, davon profitieren. Wenn ich diese Argumentation meinen Reviewern vorstelle, geben sie an, dass sie kein Problem mit meinem Refactoring haben, solange es nicht in derselben CR enthalten ist. Ich behaupte jedoch, dass dies ein Mythos ist:

(1) Meistens fällt Ihnen erst mitten in Ihrem Auftrag ein, was und wie Sie umgestalten möchten. Wie Martin Fowler es ausdrückt:

Wenn Sie die Funktionalität hinzufügen, stellen Sie fest, dass ein Teil des Codes, den Sie hinzufügen, eine Verdoppelung mit einem vorhandenen Code enthält. Sie müssen daher den vorhandenen Code überarbeiten, um die Dinge zu bereinigen Besser, wenn die Interaktion mit vorhandenen Klassen geändert wurde. Nutzen Sie diese Gelegenheit, bevor Sie sich für erledigt halten.

(2) Niemand wird Sie positiv betrachten, wenn Sie CRs "refactoring" veröffentlichen, die Sie eigentlich nicht tun sollten. Ein CR hat einen gewissen Aufwand und Ihr Manager möchte nicht, dass Sie Ihre Zeit mit dem Refactoring verschwenden. Dieses Problem wird minimiert, wenn es mit der Änderung gebündelt ist, die Sie durchführen sollen.

Das Problem wird durch Resharper noch verschärft, da jede neue Datei, die ich der Änderung hinzufüge (und ich kann nicht im Voraus genau wissen, welche Dateien sich ändern würden), normalerweise mit Fehlern und Vorschlägen übersät ist - die meisten davon sind genau richtig und absolut verdient Festsetzung.

Das Endergebnis ist, dass ich schrecklichen Code sehe und ihn einfach dort lasse. Ironischerweise habe ich das Gefühl, dass das Korrigieren eines solchen Codes nicht nur mein Ansehen verbessert, sondern es tatsächlich senkt und mich als den "unkonzentrierten" Kerl auszeichnet, der Zeit damit verschwendet, Dinge zu korrigieren, die niemand interessiert, anstatt seine Arbeit zu erledigen. Ich fühle mich schlecht dabei, weil ich schlechten Code wirklich verachte und es nicht ertrage, ihn anzusehen, geschweige denn von meinen Methoden abzurufen!

Irgendwelche Gedanken darüber, wie ich diese Situation beheben kann?

t0x1n
quelle
40
Ich würde mich your manager doesn't want you to "waste your time" on refactoring
unwohl
19
Zusätzlich zu den mehreren CRs muss jedes Commit einem einzigen Zweck dienen: einem für den Refactor, einem für die Anforderung / bug / etc. Auf diese Weise kann eine Überprüfung zwischen dem Refaktor und der angeforderten Codeänderung unterscheiden. Ich würde auch argumentieren, dass der Refactor nur durchgeführt werden sollte, wenn es Einheitentests gibt, die beweisen, dass Ihr Refactor nichts kaputt gemacht hat (Onkel Bob stimmt zu).
2
@ t0x1n Ich sehe das nicht anders
Daenyth
2
@ t0x1n yep, ich habe das verpasst. Nicht genug Kaffee heute Morgen. Meiner Erfahrung nach gibt es einige Möglichkeiten zur Umgestaltung. Vielleicht sehen Sie sich den Code an, den Sie ändern müssen, und wissen sofort, dass er aufgeräumt werden muss, also tun Sie das zuerst. Vielleicht Sie haben etwas , um Refactoring Ihre Änderung zu machen , weil die neue Anforderung mit dem vorhandenen Code nicht kompatibel ist. Ich würde behaupten, der Refaktor ist an sich Teil Ihrer Änderung und sollte nicht als getrennt betrachtet werden. Schließlich sehen Sie vielleicht, dass der Code nach der Hälfte Ihrer Änderung nicht mehr funktioniert, aber Sie können ihn beenden. Refactor nach der Tatsache.
6
Punkt 1 behauptet nicht, dass eine Trennung der Commits unmöglich ist. Es bedeutet nur, dass Sie nicht wissen, wie es geht, oder dass Ihr VCS es schwierig macht. Ich mache das die ganze Zeit, nehme sogar ein einzelnes Commit und teile es nachträglich auf.
Useless

Antworten:

18

OK, jetzt gibt es hier mehr Dinge, als für einen Kommentar geeignet sind.

tl; dr

Ihre Intuition über das, was Sie sollten tun (Refactoring as you go) ist richtig.

Ihre Schwierigkeit, dies zu implementieren - da Sie ein schlechtes Codeüberprüfungssystem umgehen müssen - liegt in Ihrer Schwierigkeit, Ihren Quellcode und Ihr VCS zu manipulieren. Mehrere Personen haben gesagt, dass Sie Ihre Änderungen (ja, sogar innerhalb von Dateien) in mehrere Commits aufteilen können und sollten , aber Sie scheinen Schwierigkeiten zu haben, dies zu glauben.

Das kannst du wirklich . Das schlagen wir wirklich vor . Sie sollten wirklich lernen, das Beste aus Ihren Werkzeugen für Bearbeitung, Quellenmanipulation und Versionskontrolle herauszuholen. Wenn Sie die Zeit investieren, um zu lernen, wie man sie richtig einsetzt, erleichtert dies das Leben erheblich.


Workflow / büropolitische Themen

Ich würde im Wesentlichen die gleiche Empfehlung wie GlenH7 aussprechen, dass Sie zwei Commits erstellen - eines mit nur Refactorings und (nachweislich oder offensichtlich) ohne funktionale Änderungen und eines mit den fraglichen funktionalen Änderungen.

Wenn Sie jedoch viele Fehler finden, kann es hilfreich sein, eine einzelne Fehlerkategorie auszuwählen, die in einer einzelnen CR behoben werden soll. Dann haben Sie ein Commit mit einem Kommentar wie "Deduplizierungscode", "Typ-X-Fehler beheben" oder was auch immer. Da es sich hierbei um eine einzelne Art von Änderung handelt, vermutlich an mehreren Stellen, sollte die Überprüfung trivial sein . Es bedeutet, dass Sie nicht jeden Fehler beheben können, den Sie finden, aber es kann weniger schmerzhaft sein, ihn durchzuschmuggeln.


VCS-Probleme

Das Aufteilen der an Ihrer Arbeitsquelle vorgenommenen Änderungen in mehrere Festschreibungen sollte keine Herausforderung sein. Sie haben nicht gesagt, was Sie verwenden, aber mögliche Workflows sind:

  1. Wenn Sie Git verwenden, haben Sie ausgezeichnete Werkzeuge dafür

    • Sie können git add -ifür die interaktive Bereitstellung über die Befehlszeile verwenden
    • Sie können git guieinzelne Hunks und Linien verwenden und auswählen, um sie zu inszenieren (dies ist eines der wenigen VCS-bezogenen GUI-Tools, die ich der Befehlszeile vorziehe, das andere ist ein guter 3-Wege-Merge-Editor).
    • Sie können viele kleine Änderungen vornehmen (einzelne Änderungen oder die Behebung derselben Fehlerklasse an mehreren Stellen) und diese dann neu anordnen, zusammenführen oder aufteilen rebase -i
  2. Wenn Sie Git nicht verwenden, verfügt Ihr VCS möglicherweise noch über Tools für diese Art von Dingen, aber ich kann keine Empfehlungen abgeben, ohne zu wissen, welches System Sie verwenden

    Sie erwähnten, dass Sie TFS verwenden - was meiner Meinung nach seit TFS2013 mit Git kompatibel ist. Es kann sich lohnen, mit einem lokalen Git-Klon des Repos zu experimentieren, um darin zu arbeiten. Wenn dies deaktiviert ist oder bei Ihnen nicht funktioniert, können Sie die Quelle trotzdem in ein lokales Git-Repo importieren, dort arbeiten und es verwenden Exportieren Sie Ihre endgültigen Commits.

  3. Sie können dies in jedem VCS manuell tun, wenn Sie Zugriff auf grundlegende Tools wie diffund haben patch. Es ist schmerzhafter, aber sicherlich möglich. Der grundlegende Workflow wäre:

    1. Nehmen Sie alle Änderungen, Tests usw. vor.
    2. Verwenden Sie diffdiese Option, um eine (kontextbezogene oder einheitliche) Patch-Datei mit allen Änderungen seit dem letzten Festschreiben zu erstellen
    3. Teilen Sie das in zwei Dateien auf: Am Ende erhalten Sie eine Patch-Datei mit Refactorings und eine mit funktionalen Änderungen
      • Dies ist nicht ganz trivial - Tools wie der Emacs Diff-Modus können hilfreich sein
    4. alles sichern
    5. Zum letzten Commit zurückkehren, patcheine der Patch-Dateien erneut abspielen und die Änderungen festschreiben
      • NB. Wenn der Patch nicht ordnungsgemäß angewendet wurde, müssen Sie die ausgefallenen Hunks möglicherweise manuell beheben
    6. Wiederholen Sie 5 für die zweite Patch-Datei

Jetzt haben Sie zwei Festschreibungen, bei denen Ihre Änderungen entsprechend partitioniert sind.

Beachten Sie, dass es wahrscheinlich Tools gibt, die das Patch-Management vereinfachen. Ich verwende sie nur nicht, da git es für mich erledigt.

Nutzlos
quelle
Ich bin mir nicht sicher, ob ich folge - geht bullet 3.3 davon aus, dass sich die Refactoring- und Funktionsänderungen in verschiedenen Dateien befinden? Jedenfalls nicht. Vielleicht macht es mehr Sinn, durch Linien zu trennen, aber ich denke nicht, dass wir in unserem CVS (TFS) Werkzeuge dafür haben. Auf jeden Fall würde es für viele (die meisten?) Umgestaltungen nicht funktionieren, bei denen die funktionalen Änderungen von den umgestalteten Änderungen abhängen. Nehmen wir zum Beispiel an, dass ich die Refaktormethode Foo (die ich als Teil meiner Funktionsänderung verwenden muss) verwende, um 3 statt 2 Parameter zu verwenden.
t0x1n
1
Unterschiedliche Zeilen in derselben Datei sind in den angegebenen Workflows in Ordnung. Und da die beiden Festschreibungen sequentiell sind , ist es völlig in Ordnung, dass die zweite (funktionale) Festschreibung von der ersten abhängt. Oh, und TFS2013 unterstützt angeblich Git.
Nutzlos
Wir verwenden auch TFS für die Quellcodeverwaltung. Sie gehen davon aus, dass das zweite Commit das funktionale Commit ist, während es normalerweise das Gegenteil ist (da ich vorher nicht sagen kann, was für ein Refactoring durchgeführt werden muss). Ich nehme an, ich könnte meine ganze Arbeit mit funktionalem + Refactoring machen, dann alles Funktionale loswerden und es in einem separaten Commit wieder hinzufügen. Ich sage nur, das ist eine Menge Aufwand (und Zeit), nur um ein paar Rezensenten glücklich zu machen. Meiner Meinung nach ist es vernünftiger, opportunistisches Refactoring zuzulassen und im Gegenzug CR-Änderungen selbst zuzustimmen (unter Berücksichtigung der zusätzlichen Schwierigkeit).
t0x1n
3
Ich glaube, du verstehst mich wirklich nicht. Das Bearbeiten von Quellen und das Gruppieren von Bearbeitungen in Festschreibungen, ja sogar Bearbeitungen in derselben Datei, sind logisch getrennte Aktivitäten. Wenn dies schwierig erscheint, müssen Sie nur die verfügbaren Tools zur Quellcodeverwaltung besser erlernen.
Nutzlos
1
Ja, Ihr Verständnis ist korrekt, Sie hätten zwei aufeinanderfolgende Commits, wobei das zweite (funktionsfähig) vom ersten abhängt (Refactoring). Der oben beschriebene Diff / Patch-Workflow ist genau eine Möglichkeit, dies zu tun, ohne dass Änderungen manuell gelöscht und anschließend neu geschrieben werden müssen.
Nutzlos
29

Ich gehe davon aus, dass Change Requests in Ihrem Unternehmen groß und formell sind. Wenn nicht, nehmen Sie die Änderungen (soweit möglich) in viele kleine Commits vor (wie Sie es sollten).

Irgendwelche Gedanken darüber, wie ich diese Situation beheben kann?

Mach weiter was du tust?

Ich meine, alle Ihre Gedanken und Schlussfolgerungen sind völlig richtig. Sie sollten Dinge reparieren, die Sie sehen. Die Leute machen nicht genug geplantes Refactoring. Und dieser Nutzen für die gesamte Gruppe ist wichtiger, als einige zu belasten.

Was helfen kann, ist weniger kämpferisch zu sein. Code-Reviews sollten nicht kämpferisch sein "Warum hast du das getan?", Sie sollten kooperativ sein "Hey Leute, während ich hier war, habe ich all diese Dinge repariert!". Die Zusammenarbeit (mit Ihrem Lead / Manager, falls möglich), um diese Kultur zu ändern, ist schwierig, aber sehr wichtig, um ein gut funktionierendes Entwicklungsteam zu schaffen.

Sie können auch (wenn möglich mit Ihrem Lead / Manager) zusammenarbeiten, um gemeinsam mit Ihren Kollegen die Wichtigkeit dieser Ideen zu fördern. Lassen Sie sich fragen: "Warum interessiert ihr euch nicht für Qualität?" anstatt sie zu fragen "warum machst du immer diese nutzlosen Dinge?".

Telastyn
quelle
5
Ja, CRs sind groß und formal. Änderungen werden per CRed abgemeldet und dann in eine Warteschlange gestellt. Kleine Änderungen sollten als Iterationen zu einem laufenden CR hinzugefügt werden, anstatt separat festgeschrieben zu werden. WRT macht weiter mit dem, was ich tue, das mag zwar der Gruppe nützen, aber ich fürchte, es wird mir nicht nützen . Diese Leute, denen ich "Unannehmlichkeiten" bereite, sind wahrscheinlich die gleichen Leute, die mich in der jährlichen Rezension bewerten würden. Das Problem bei der Veränderung der Kultur ist, dass die großen Chefs daran glauben. Vielleicht muss ich mir mehr Respekt in ihren Augen verdienen, bevor ich so etwas versuche ...
t0x1n
13
@ t0x1n - Schau mich nicht an. Ich habe eine Karriere gemacht, bei Leuten, die hartnäckig dem Saugen verpflichtet sind, das Richtige zu tun. Vielleicht nicht so profitabel wie ich es getan hätte, wenn ich die Menschen glücklich gemacht hätte, aber ich schlafe gut.
Telastyn
Danke, dass du ehrlich bist. Es ist in der Tat etwas zum Nachdenken.
t0x1n
1
Ich stoße auch oft darauf. Sicherzustellen, dass ich einen "Aufräum" -Patch habe, und dann hilft ein Arbeitspatch sehr. Normalerweise kämpfe ich im System und lasse mich dann an einem weniger stressigen Ort nieder. Das heißt, es gibt manchmal triftige Gründe für die Bedenken Ihrer Mitarbeiter. Zum Beispiel, wenn Code schnell in Produktion geht und es keine ausreichenden Tests gibt. Ich habe die Codeüberprüfung als Versuch gesehen, das Testen zu vermeiden. Es funktioniert nicht. Durch die Codeüberprüfung bleibt der Code einheitlich. Es tut wenig gegen Bugs.
Sean Perry
1
@ SeanPerry einverstanden - aber ich spreche über normale Umstände, in denen Tests existieren, Fehlerbashes durchgeführt werden usw.
t0x1n
14

Ich habe viel Verständnis für Ihre Situation, aber nur wenige konkrete Vorschläge. Wenn nichts anderes, kann ich Sie vielleicht davon überzeugen, dass es so schlimm ist, wie es ist, könnte es schlimmer sein. Es kann IMMER schlimmer sein. :-)

Erstens denke ich, dass Sie (mindestens) zwei Probleme mit Ihrer Kultur haben, nicht nur eines. Ein Problem ist die Herangehensweise an das Refactoring, aber die Codeüberprüfungen scheinen ein eindeutiges Problem zu sein. Ich werde versuchen, meine Gedanken zu trennen.

Code-Überprüfungen

Ich war in einer Gruppe, die Code-Reviews hasste. Die Gruppe wurde gebildet, indem zwei Gruppen aus verschiedenen Teilen des Unternehmens zusammengeführt wurden. Ich stamme aus der Gruppe, die seit mehreren Jahren Code-Überprüfungen mit im Allgemeinen guten Ergebnissen durchführt. Die meisten von uns waren der Meinung, dass Code-Reviews unsere Zeit gut nutzen. Wir schlossen uns zu einer größeren Gruppe zusammen und so gut wir konnten, hatte diese "andere" Gruppe noch nie von Code-Reviews gehört. Jetzt arbeiteten wir alle an "ihrer" Codebasis.

Die Dinge waren nicht gut, als wir fusionierten. Die neuen Funktionen verspäteten sich Jahr für Jahr um 6-12 Monate. Der Bug-Backlog war riesig, größer und lebensentziehend. Die Code-Inhaberschaft war stark, insbesondere unter den ältesten "Gurus". "Feature Branches" dauerten manchmal Jahre und erstreckten sich über einige Veröffentlichungen. manchmal sah NIEMAND, aber ein einzelner Entwickler den Code, bevor er den Hauptzweig erreichte. Tatsächlich ist "Feature Branch" irreführend, da dies darauf hindeutet, dass sich der Code irgendwo im Repository befindet. Häufig war es nur auf dem individuellen System des Entwicklers.

Das Management war sich einig, dass wir "etwas tun" müssen, bevor die Qualität unannehmbar niedrig wird. :-) Ihre Antwort war Code Reviews. Code Reviews wurde zu einem offiziellen "Thou Shalt" -Element, das jedem Check-in vorausgeht. Das von uns verwendete Tool war Review Board.

Wie hat es in der von mir beschriebenen Kultur funktioniert? Besser als nichts, aber es war schmerzhaft und es dauerte mehr als ein Jahr, bis ein Mindestmaß an Compliance erreicht war. Einige Dinge, die wir beobachtet haben:

  1. Das von Ihnen verwendete Tool fokussiert Codeüberprüfungen auf bestimmte Arten. Dies kann ein Problem sein. Review Board bietet Ihnen nette, farbenfrohe zeilenweise Unterschiede und ermöglicht es Ihnen, Kommentare zu einer Zeile hinzuzufügen. Dies führt dazu, dass die meisten Entwickler alle Zeilen ignorieren, die sich nicht geändert haben. Das ist in Ordnung für kleine, isolierte Änderungen. Es ist nicht so gut für große Änderungen, große Mengen an neuem Code oder Code mit 500 Instanzen einer umbenannten Funktion, gemischt mit 10 Zeilen neuer Funktionalität.
  2. Obwohl wir uns in einer alten, kranken Codebasis befanden, die größtenteils noch nie zuvor überprüft worden war, wurde es für einen Überprüfer "unhöflich", zu etwas Stellung zu nehmen, das KEINE Änderungszeile war, selbst um auf einen offensichtlichen Fehler hinzuweisen. "Stört mich nicht, dies ist ein wichtiger Check-in und ich habe keine Zeit, um Fehler zu beheben."
  3. Suchen Sie nach einem "einfachen" Rezensenten. Einige Leute sehen sich eine 10-Dateien-Überprüfung mit 2000 geänderten Zeilen für 3 Minuten an und klicken auf "Versenden!". Jeder lernt schnell, wer diese Leute sind. Wenn Sie wirklich nicht möchten, dass Ihr Code zuerst überprüft wird, senden Sie ihn an einen "einfachen" Prüfer. Ihr Check-in wird nicht verlangsamt. Sie können den Gefallen erwidern, indem Sie ein "einfacher" Prüfer für seinen Code werden.
  4. Wenn Sie Code-Reviews hassen, ignorieren Sie einfach die E-Mails, die Sie vom Review Board erhalten. Ignorieren Sie die Folgemaßnahmen Ihrer Teammitglieder. Für Wochen. Bis Sie 3 Dutzend offene Überprüfungen in Ihrer Warteschlange haben und Ihr Name einige Male in Gruppensitzungen auftaucht. Dann werden Sie ein "einfacher" Bewerter und löschen Sie alle Ihre Bewertungen vor der Mittagszeit.
  5. Senden Sie Ihren Code nicht an einen "harten" Prüfer. (Die Art von Entwickler, der eine Frage wie diese stellt oder beantwortet.) Jeder lernt schnell, wer die "harten" Reviewer sind, genauso wie er die einfachen lernt. Wenn Codeüberprüfungen Zeitverschwendung sind, ist das Lesen von detailliertem Feedback zu dem Code, den Sie besitzen, sowohl Zeitverschwendung als auch eine Beleidigung.
  6. Wenn Codeüberprüfungen schmerzhaft sind, setzen die Leute sie ab und schreiben immer mehr Code. Sie erhalten weniger Codeüberprüfungen, aber jede ist GROSS. Sie benötigen mehr, kleinere Codeüberprüfungen, was bedeutet, dass das Team herausfinden muss, wie sie so schmerzfrei wie möglich gestaltet werden können.

Ich dachte, ich würde ein paar Absätze über Code Reviews schreiben, aber es stellte sich heraus, dass ich hauptsächlich über Kultur schrieb. Ich denke, es läuft darauf hinaus: Code-Reviews können für ein Projekt gut sein, aber ein Team bekommt nur die Vorteile, die es verdient.

Refactoring

Hat meine Gruppe das Refactoring noch mehr verachtet, als es Code-Reviews hasste? Na sicher! Aus all den offensichtlichen Gründen, die bereits erwähnt wurden. Sie verschwenden meine Zeit mit einer icky Codeüberprüfung, und Sie fügen nicht einmal eine Funktion hinzu oder beheben einen Fehler!

Aber der Code musste noch dringend überarbeitet werden. Wie geht es weiter?

  1. Mischen Sie niemals eine Umgestaltungsänderung mit einer Funktionsänderung. Einige Leute haben diesen Punkt erwähnt. Wenn Codeüberprüfungen ein Reibungspunkt sind, erhöhen Sie die Reibung nicht. Es ist mehr Arbeit, aber Sie sollten eine separate Überprüfung und einen separaten Check-in einplanen.
  2. Fangen Sie klein an. Sehr klein. Noch kleiner als das. Verwenden Sie sehr kleine Refactorings, um den Leuten schrittweise beizubringen, dass Refactoring und Code-Reviews nicht nur böse sind. Wenn Sie sich in einem winzigen Refactoring pro Woche ohne allzu große Schmerzen zurechtfinden, könnten Sie in ein paar Monaten mit zwei pro Woche davonkommen. Und sie können etwas größer sein. Besser als nichts.
  3. Wir hatten im Wesentlichen keine Unit-Tests. Refactoring ist also verboten, oder? Nicht unbedingt. Bei einigen Änderungen ist der Compiler Ihr Komponententest. Konzentrieren Sie sich auf Refactorings, die Sie testen können. Vermeiden Sie Änderungen, wenn diese nicht getestet werden können. Vielleicht verbringen Sie die Zeit damit, Unit-Tests zu schreiben.
  4. Einige Entwickler haben Angst vor Umgestaltungen, weil sie Angst vor ALLEN Codeänderungen haben. Ich habe lange gebraucht, um diesen Typ zu erkennen. Sie schreiben einen Teil des Codes, fummeln daran herum, bis er "funktioniert", und möchten ihn dann NIEMALS ändern. Sie verstehen nicht unbedingt, warum es funktioniert. Veränderung ist riskant. Sie vertrauen nicht auf sich selbst, um eine Veränderung herbeizuführen, und sie werden Ihnen sicherlich nicht vertrauen. Beim Refactoring soll es um kleine, sichere Änderungen gehen, die das Verhalten nicht ändern. Es gibt Entwickler , für die die Idee einer „Änderung , die nicht Verhalten nicht ändert“ ist undenkbar . Ich weiß nicht, was ich in diesen Fällen vorschlagen soll. Ich denke, man muss versuchen, in Bereichen zu arbeiten, die ihnen egal sind. Ich war überrascht, als ich erfuhr, dass dieser Typ lange haben kann,
GraniteRobert
quelle
1
Dies ist eine super durchdachte Antwort, danke! Ich stimme insbesondere zu, wie das CR-Tool den Fokus der Überprüfung beeinflussen kann. Zeile für Zeile ist der einfache Ausweg, bei dem nicht wirklich verstanden wird, was vorher und was jetzt passiert. Und natürlich ist der Code, der sich nicht geändert hat, perfekt, das muss man sich nicht ansehen ...
t0x1n
1
" Könnte schlimmer sein. Könnte regnen. " Als ich Ihren letzten Absatz las (zweiter Punkt # 4), dachte ich: Sie brauchen mehr Überprüfung, Kodiererüberprüfung . Und einige Umgestaltungen, wie in: "yourSalary = 0"
Absolut perfekt an so vielen Fronten, unglaubliche Antwort. Ich kann total sehen, woher du kommst: Ich bin selbst in genau der gleichen Situation und es ist unglaublich frustrierend. Sie sind in diesem ständigen Kampf um Qualität und bewährte Praktiken und es gibt keine Unterstützung von nicht nur dem Management, sondern INSBESONDERE auch von anderen Entwicklern im Team.
Julealgon
13

Warum machst du nicht beides, aber mit separaten Commits?

Ihre Kollegen haben einen Punkt. Eine Codeüberprüfung sollte den Code auswerten, der von einer anderen Person geändert wurde. Sie sollten den Code, den Sie für eine andere Person überprüfen, nicht berühren, da dies Ihre Rolle als Überprüfer beeinträchtigt.

Wenn Sie jedoch eine Reihe eklatanter Probleme feststellen, haben Sie zwei Möglichkeiten.

  1. Wenn die Codeüberprüfung ansonsten in Ordnung war, lassen Sie zu, dass der von Ihnen überprüfte Teil festgeschrieben wird, und überarbeiten Sie den Code dann bei einem zweiten Check-in.

  2. Wenn die Codeüberprüfung Probleme aufwies, die behoben werden mussten, fordern Sie den Entwickler auf, basierend auf den Resharper-Empfehlungen eine Umgestaltung durchzuführen.


quelle
Ich entnehme Ihrer Antwort, dass Sie an eine starke Code-Eigentümerschaft glauben. Ich empfehle Ihnen, Fowlers Gedanken darüber zu lesen, warum dies eine schlechte Idee ist: martinfowler.com/bliki/CodeOwnership.html . Um Ihre Punkte spezifisch anzusprechen: (1) ist ein Mythos, da Refactoring während der Durchführung Ihrer Änderung stattfindet - nicht davor oder danach in einer separaten, sauberen, nicht zusammenhängenden Art und Weise, wie es separate CRs erfordern würden. (2) Mit den meisten Entwicklern wird es niemals passieren. Niemals . Die meisten Entwickler interessieren sich nicht für diese Dinge, geschweige denn für jemanden, der nicht ihr Manager ist. Sie haben ihre eigenen Sachen, die sie machen wollen.
t0x1n
8
@ t0x1n: Wenn sich Ihre Manager nicht um das Refactoring kümmern und sich Ihre Kollegen nicht um das Refactoring kümmern, dann sinkt das Unternehmen langsam. Weglaufen! :)
logc
8
@ t0x1n nur weil Sie die Änderungen zusammen machen, bedeutet nicht , Sie haben zu begehen sie zusammen. Außerdem ist es oft nützlich , um Ihr Refactoring hatte keine unerwarteten Nebenwirkungen zu überprüfen separat von der Überprüfung Ihrer Funktionsänderung die erwartete Wirkung hatte. Dies gilt natürlich nicht für alle Refactorings, ist aber im Allgemeinen kein schlechter Ratschlag.
Nutzlos
2
@ t0x1n - Ich kann mich nicht erinnern, etwas über starken Code-Besitz gesagt zu haben. Meine Antwort war es, Ihre Rolle als Rezensent rein zu halten. Wenn ein Prüfer Änderungen einführt, ist er nicht mehr nur ein Prüfer.
3
@ GlenH7 Vielleicht gab es hier ein Missverständnis - ich bin nicht der Rezensent. Ich programmiere nur das, was ich brauche, und stoße auf Code, den ich dabei verbessern kann. Meine Rezensenten beschweren sich dann, wenn ich es tue.
t0x1n
7

Ich persönlich hasse die Idee von Post-Commit-Code-Überprüfungen. Die Codeüberprüfung sollte stattfinden, während Sie die Codeänderungen vornehmen. Ich spreche hier natürlich von Pair-Programmierung. Wenn Sie koppeln, erhalten Sie die Überprüfung kostenlos und Sie erhalten eine bessere Codeüberprüfung. Jetzt drücke ich hier meine Meinung aus, ich weiß, dass andere dies teilen, wahrscheinlich gibt es Studien, die dies belegen.

Wenn Sie Ihre Codeüberprüfer dazu bringen können, sich mit Ihnen zu paaren, sollte das kämpferische Element der Codeüberprüfung verschwinden. Wenn Sie eine Änderung vornehmen, die nicht verstanden wird, kann die Frage an der Stelle der Änderung gestellt, diskutiert und nach Alternativen gesucht werden, die zu besseren Refactorings führen können. Sie erhalten eine Codeüberprüfung mit höherer Qualität, da das Paar den größeren Umfang der Änderung verstehen kann und sich nicht so sehr auf zeilenweise Änderungen konzentriert, wie dies beim Vergleichen von Code nebeneinander der Fall ist.

Dies ist natürlich keine direkte Antwort auf das Problem, dass Refactorings außerhalb des Bereichs der Änderungen liegen, an denen gearbeitet wird, aber ich würde davon ausgehen, dass Ihre Kollegen die Gründe für die Änderungen besser verstehen, wenn sie dort waren, als Sie die Änderung vorgenommen haben.

Abgesehen davon, dass Sie TDD oder eine Form von rot-grünem Refaktor durchführen, können Sie mithilfe der Ping-Pong-Pairing-Technik sicherstellen, dass sich Ihre Kollegen verloben. Einfach erklärt, wie der Treiber für jeden Schritt des RGR-Zyklus gedreht wird, dh Paar 1 schreibt einen Fehlertest, Paar 2 repariert ihn, Paar 1 Refaktoren, Paar 2 schreibt einen Fehlertest ... und so weiter.

daffers
quelle
Hervorragende Punkte. Leider bezweifle ich aufrichtig, dass ich "das System" ändern kann. Manchmal kommen die Rezensenten auch aus verschiedenen Zeitzonen und geografischen Regionen. In solchen Fällen fliegt es also nicht, unabhängig davon.
t0x1n
6

Wahrscheinlich spiegelt dieses Problem ein viel größeres Problem mit der Organisationskultur wider. Die Leute scheinen mehr daran interessiert zu sein, "seine Arbeit" richtig zu machen, als das Produkt zu verbessern. Wahrscheinlich hat dieses Unternehmen eine "Schuld" -Kultur anstelle einer kollektiven Kultur und die Leute scheinen mehr daran interessiert zu sein, sich selbst zu schützen, als eine ganze Produkt- / Firmenvision zu haben .

Meiner Meinung nach haben Sie vollkommen recht. Die Leute, die Ihren Code überprüfen, sind völlig falsch, wenn sie sich beschweren, weil Sie Dinge "anfassen", versuchen, diese Leute zu überzeugen, aber niemals gegen Ihre Prinzipien verstoßen Die wichtigste Qualität eines echten Profis.

Und wenn das Richtige Ihnen schlechte Zahlen in irgendeiner dummen Art und Weise liefert, um Ihre Arbeit zu bewerten, was ist das Problem ?, wer möchte dieses Bewertungsspiel in einer verrückten Firma "gewinnen" ?, versuchen Sie es zu ändern !, und wenn es unmöglich ist oder Sie sind müde, suchen einen anderen Ort, aber niemals, niemals gegen Ihre Prinzipien, ist das Beste, was Sie haben.

AlfredoCasado
quelle
1
Sie möchten das Bewertungsspiel gewinnen, sobald Sie feststellen, dass es Sie mit Gehaltserhöhungen, Boni und Aktienoptionen belohnt :)
t0x1n
2
Nein. Sie tauschen Geld gegen Prinzipien @ t0x1n. So einfach ist das. Sie haben drei Möglichkeiten: Arbeiten, um das System zu reparieren, das System zu akzeptieren und das System zu verlassen. Die Optionen 1 und 3 sind gut für die Seele.
Sean Perry
2
Ein Unternehmen mit Prinzipien, die sich auf lokale Maximen konzentrieren, ist normalerweise weniger optimal als ein Unternehmen, das sich auf globale Maximen konzentriert. Nicht nur das, Arbeit ist nicht nur Geld, Sie verbringen jeden Tag viel Zeit in Ihrer Arbeit, fühlen sich wohl bei Ihrer Arbeit, fühlen, dass Sie die richtigen Dinge tun, wahrscheinlich ist es viel besser, als ein Haufen mehr Dollar pro Monat.
AlfredoCasado
1
Meh, Seelen sind überbewertet;)
t0x1n
2
@ SeanPerry Ich habe wirklich versucht, das System zu reparieren, aber es war sehr schwer. (1) Ich war in dieser Sache praktisch allein und es ist schwer, gegen die großen Häuptlinge vorzugehen (ich bin nur ein regulärer Entwickler, nicht einmal älter). (2) Diese Dinge brauchen Zeit, die ich einfach nicht hatte. Es gibt viel Arbeit und die gesamte Umgebung ist sehr zeitaufwendig (E-Mails, Unterbrechungen, CRs, fehlerhafte Testzyklen, die Sie reparieren müssen, Besprechungen usw. usw.). Ich gebe mein Bestes, um zu filtern und produktiv zu sein, aber normalerweise kann ich meine "vorgeschriebenen" Arbeiten kaum rechtzeitig beenden (hohe Standards helfen hier nicht), geschweige denn daran, das System zu ändern ...
t0x1n 13.06.14
5

Manchmal ist Refactoring eine schlechte Sache. Nicht aus den Gründen, die Ihre Code-Prüfer angeben; es hört sich so an, als ob sie sich nicht wirklich für die Qualität des Codes interessieren, und Sie kümmern sich darum, was großartig ist. Es gibt jedoch zwei Gründe, die Sie vom Refactoring abhalten sollten : 1. Sie können die vorgenommenen Änderungen nicht mit automatisierten Tests (Komponententests oder was auch immer) testen. 2. Sie nehmen Änderungen an einem Code vor, den Sie nicht sehr verstehen Gut; Das heißt, Sie haben nicht das domänenspezifische Wissen, um zu wissen, welche Änderungen Sie vornehmen sollten .

1. Refaktorieren Sie nicht, wenn Sie die vorgenommenen Änderungen nicht mit automatisierten Tests testen können.

Die Person, die Ihre Codeüberprüfung durchführt, muss sicher sein können, dass die von Ihnen vorgenommenen Änderungen nichts kaputtgemacht haben, was früher funktioniert hat. Dies ist der Hauptgrund, den Arbeitscode in Ruhe zu lassen. Ja, es wäre definitiv besser, diese 1000 Zeilen lange Funktion (das ist wirklich ein Gräuel!) In eine Reihe von Funktionen umzuwandeln, aber wenn Sie Ihre Tests nicht automatisieren können, kann es sehr schwierig sein, andere davon zu überzeugen, dass Sie alles getan haben richtig. Ich habe diesen Fehler definitiv schon einmal gemacht.

Stellen Sie vor dem Refactoring sicher, dass Unit-Tests durchgeführt wurden. Wenn es keine Komponententests gibt, schreibe welche! Dann umgestalten, und Ihre Code-Prüfer werden keine (legitime) Entschuldigung dafür haben, aufgebracht zu sein.

Refaktorieren Sie keine Codeteile, die domänenspezifisches Wissen erfordern, über das Sie nicht verfügen.

Der Ort, an dem ich arbeite, enthält viel Code, der sich mit Chemieingenieurwesen befasst. Die Codebasis verwendet die für Chemieingenieure gebräuchlichen Redewendungen. Nehmen Sie niemals Änderungen vor, die für ein Feld typisch sind. Es mag wie eine gute Idee scheint eine Variable mit dem Namen umbenennen xzu molarConcentration, aber was denken Sie? In allen Chemietexten ist die molare Konzentration mit einem dargestellt x. Durch das Umbenennen wird es schwieriger zu wissen, was im Code für Personen in diesem Bereich tatsächlich vor sich geht.

Anstatt idiomatische Variablen umzubenennen, geben Sie einfach Kommentare ein, in denen Sie erklären, was sie sind. Wenn sie nicht idiomatisch sind, benennen Sie sie bitte um. Lassen Sie nicht die i, j, k, x, y, usw. herum schwimmen , wenn bessere Namen arbeiten.

Faustregel für Abkürzungen: Wenn Personen im Gespräch eine Abkürzung verwenden, ist es in Ordnung, diese Abkürzung im Code zu verwenden. Beispiele aus der Codebasis bei der Arbeit:

Wir haben die folgenden Konzepte, die wir immer abkürzen, wenn wir über sie sprechen: "Area of ​​Concern" wird zu "AOC", "Vapor Cloud Explosion" wird zu VCE, so etwas. In unserer Codebasis hat jemand alle Instanzen überarbeitet, die als aoc für AreaOfConcern, VCE für vaporCloudExplosion, nPlanes für ConfiningPlanesCount ... bezeichnet wurden, was das Lesen des Codes für Benutzer mit domänenspezifischen Kenntnissen erheblich erschwert hat. Ich habe mich auch schuldig gemacht, solche Dinge getan zu haben.


Dies mag in Ihrer Situation möglicherweise überhaupt nicht zutreffen, aber ich habe einige Gedanken zu diesem Thema.

Cody Piersall
quelle
Vielen Dank, Cody. In Bezug auf "Ihre Code-Prüfer werden keine (legitime) Entschuldigung haben, um verärgert zu sein" - ihre Entschuldigung ist bereits unzulässig, da sie sich darüber ärgern, dass es schwieriger ist, die Änderungen durchzugehen, als über Korrektheit, Lesbarkeit oder ähnliches.
t0x1n
2

Diese Frage hat jetzt zwei unterschiedliche Probleme: die einfache Überprüfung von Änderungen bei Codeüberprüfungen und den Zeitaufwand für das Refactoring.

Wo ich arbeite, gibt es eine Entwicklungspraxis, die starke Reibung verursacht - Code Review (CR). Immer wenn ich etwas ändere, das nicht im Rahmen meiner "Aufgabe" liegt, werde ich von meinen Gutachtern zurechtgewiesen, dass ich die Änderung schwieriger zu überprüfen mache.

Wie andere Antworten bereits sagten - können Sie die Refactoring-Checkins von den Code-Change-Checkins trennen (nicht unbedingt als separate Überprüfungen)? Abhängig von dem Tool, das Sie für die Codeüberprüfung verwenden, sollten Sie in der Lage sein, die Unterschiede nur zwischen bestimmten Revisionen anzuzeigen (Atlassian's Crucible erledigt dies definitiv).

(2) Niemand wird Sie positiv betrachten, wenn Sie "refactoring" CRs veröffentlichen, die Sie eigentlich nicht machen sollten. Ein CR hat einen gewissen Aufwand und Ihr Manager möchte nicht, dass Sie Ihre Zeit mit dem Refactoring verschwenden. Dieses Problem wird minimiert, wenn es mit der Änderung gebündelt ist, die Sie durchführen sollen.

Wenn das Refactoring unkompliziert ist und das Verstehen des Codes erleichtert (was alles ist, was Sie tun sollten, wenn es nur um das Refactoring geht), sollte die Codeüberprüfung nicht lange dauern und nur einen minimalen Aufwand bedeuten, der sich bei jemandem in Schüben zurückgewinnt muss in Zukunft noch einmal auf den Code schauen. Wenn Ihre Chefs dies nicht in der Lage sind, sollten Sie sie vorsichtig an einige Ressourcen heranführen, in denen erläutert wird, warum die Pfadfinderregel zu einer produktiveren Beziehung mit Ihrem Code führt. Wenn Sie sie davon überzeugen können, dass die "Verschwendung Ihrer Zeit" jetzt zwei-, fünf- oder zehnmal so viel spart, wenn Sie / jemand anderes zurückkommt, um mehr an diesem Code zu arbeiten, dann sollte Ihr Problem verschwinden.

Das Problem wird durch Resharper noch verschärft, da jede neue Datei, die ich der Änderung hinzufüge (und ich kann nicht im Voraus genau wissen, welche Dateien sich ändern würden), normalerweise mit Fehlern und Vorschlägen übersät ist - die meisten davon sind genau richtig und absolut verdient Festsetzung.

Haben Sie versucht, Ihre Kollegen auf diese Probleme aufmerksam zu machen und zu diskutieren, warum es sich lohnt, sie zu beheben? Und kann einer dieser Fehler automatisch behoben werden (vorausgesetzt, Sie verfügen über eine ausreichende Testabdeckung, um zu bestätigen, dass Sie mit dem automatischen Refactoring keine Fehler begangen haben)? Manchmal ist es "Ihre Zeit nicht wert", ein Refactoring durchzuführen, wenn es wirklich wählerisches Zeug ist. Verwendet Ihr gesamtes Team ReSharper? Wenn ja, haben Sie eine gemeinsame Richtlinie darüber, welche Warnungen / Regeln durchgesetzt werden? Wenn dies nicht der Fall ist, sollten Sie vielleicht erneut demonstrieren, wo das Tool Ihnen hilft, Bereiche des Codes zu identifizieren, die mögliche Ursachen für künftige Schmerzen sind.

Chris Cooper
quelle
In Bezug auf die CR-Trennung habe ich in meinem Beitrag und einigen anderen Kommentaren darauf hingewiesen, warum ich das für unmöglich halte.
t0x1n
Ich spreche nicht von wählerischen Dingen, ich spreche von tatsächlichen Leistungs- und Korrektheitsproblemen sowie von redundantem Code, doppeltem Code usw. Und das ist nur R # -Ding, es gibt auch wirklich schlechten Code, den ich leicht reparieren kann . Leider verwenden nicht alle in meinem Team Resharper, und selbst diejenigen, die es nicht zu ernst nehmen. Es ist ein gewaltiger Bildungsaufwand erforderlich, und vielleicht werde ich versuchen, so etwas zu leiten. Es ist jedoch schwierig, da ich kaum genug Zeit für die Arbeit habe , die ich zu erledigen habe, geschweige denn für Nebenerziehungsprojekte. Vielleicht muss ich nur auf eine Ausfallzeit warten, um sie auszunutzen.
t0x1n
Ich werde mich nur einschalten und sagen, dass es definitiv nicht unmöglich ist , da ich sehe, dass es die ganze Zeit gemacht wird. Nehmen Sie die Änderungen vor, die Sie ohne das Refactoring vornehmen würden, checken Sie sie ein und refactern Sie sie, um aufzuräumen, und checken Sie sie ein. Es ist keine Hexerei. Oh, und seien Sie bereit zu verteidigen, warum es sich für Sie lohnt, den überarbeiteten Code zu überarbeiten und zu überprüfen.
Eric King
@EricKing Das könnte ich wohl machen. Allerdings: (1) Ich müsste mit hässlichem Code arbeiten und mir notieren, was ich verbessern möchte, bis ich mit den funktionalen Änderungen fertig bin, die meinen funktionalen Fortschritt stören und verlangsamen. (2) Sobald ich meine funktionalen Änderungen übermittelt habe Wenn Sie meine Notizen für das Refactoring noch einmal durchgehen, wird dies nur die erste Iteration sein, und der Abschluss des Refactorings kann mehr Zeit in Anspruch nehmen. Wie Sie vorgeschlagen haben, fällt es mir schwer, meinen Managern das zu erklären, da meine Arbeit bereits "erledigt" ist.
t0x1n
2
"Ich spreche über tatsächliche Leistungs- und Korrektheitsprobleme" - dann könnte dies die Definition von Refactoring erweitern; Wenn der Code tatsächlich falsch ist, handelt es sich um eine Fehlerbehebung. Was Leistungsprobleme betrifft, sollten Sie diese nicht nur im Rahmen einer Funktionsänderung beheben, sondern müssen sie möglicherweise messen, gründlich testen und den Code separat überprüfen.
Chris Cooper
2

Ich kann mich erinnern, dass ich vor etwa 25 Jahren Code "aufgeräumt" habe, an dem ich gerade zu anderen Zwecken gearbeitet habe. Meistens durch Neuformatierung von Kommentarblöcken und Tabulator- / Spaltenausrichtungen, um den Code übersichtlicher und damit verständlicher zu machen (keine tatsächlichen funktionalen Änderungen). . Ich mag Code, der ordentlich und ordentlich ist. Nun, die Senior-Entwickler waren wütend. Es stellte sich heraus, dass sie eine Art Dateivergleich (diff) verwendeten, um zu sehen, was sich im Vergleich zu ihren persönlichen Kopien der Datei geändert hatte, und jetzt gab es alle möglichen Fehlalarme. Ich sollte erwähnen, dass sich unsere Codebibliothek auf einem Mainframe befand und keine Versionskontrolle hatte - Sie haben im Grunde alles überschrieben, was da war, und das war es.

Die Moral der Geschichte? Ich weiß nicht. Ich denke, manchmal kann man niemandem außer sich selbst gefallen. Ich habe keine Zeit verschwendet (in meinen Augen) - der aufgeräumte Code war leichter zu lesen und zu verstehen. Es ist nur so, dass die primitiven Änderungskontrollwerkzeuge, die von anderen verwendet werden, zusätzliche einmalige Arbeit an ihnen leisten. Die Werkzeuge waren zu primitiv, um Leerzeichen / Tabulatoren und Kommentare zu ignorieren.

Phil Perry
quelle
Ja, ich werde auch wegen des Platzbedarfs und wegen trivialer Dinge wie redundantem Casting usw. abgespritzt.
t0x1n
2

Wenn Sie die angeforderte Änderung und den nicht angeforderten Refactor in (viele) separate Commits aufteilen können, wie von @Useless, @Telastyn und anderen angegeben, ist dies das Beste, was Sie tun können. Sie werden weiterhin an einem einzelnen CR arbeiten, ohne den administrativen Aufwand für das Erstellen eines "Refactorings". Halten Sie einfach Ihre Commits klein und konzentriert.

Anstatt Ihnen einige Ratschläge zu geben, bevorzuge ich es, Sie auf eine viel umfassendere Erklärung (in der Tat ein Buch) eines Spezialisten hinzuweisen. Auf diese Weise können Sie viel mehr lernen. Lesen Sie Effektiv mit Legacy-Code arbeiten (von Michael Feathers) . In diesem Buch erfahren Sie, wie Sie das Refactoring durchführen und dabei die Funktionsänderungen berücksichtigen.

Die behandelten Themen umfassen:

  • Verstehen der Mechanismen von Softwareänderungen: Hinzufügen von Funktionen, Beheben von Fehlern, Verbessern des Designs und Optimieren der Leistung
  • Legacy-Code in ein Test-Harness einbinden
  • Schreiben von Tests, die Sie vor neuen Problemen schützen
  • Techniken, die mit jeder Sprache oder Plattform verwendet werden können - mit Beispielen in Java, C ++, C und C #
  • Genau identifizieren, wo Codeänderungen vorgenommen werden müssen
  • Umgang mit Legacy-Systemen, die nicht objektorientiert sind
  • Behandlung von Anwendungen, die keine Struktur zu haben scheinen

Dieses Buch enthält auch einen Katalog mit vierundzwanzig Techniken zum Aufbrechen von Abhängigkeiten, mit denen Sie isoliert mit Programmelementen arbeiten und sicherere Änderungen vornehmen können.

Hbas
quelle
2

Auch ich glaube fest an die Boyscout-Regel und das opportunistische Refactoring. Das Problem besteht häufig darin, das Management zum Kauf zu bewegen. Refactoring ist sowohl mit Risiken als auch mit Kosten verbunden. Was das Management oft übersieht, ist, dass dies auch die technischen Schulden betrifft.

Es ist die menschliche Natur. Wir sind es gewohnt, mit echten Problemen umzugehen und sie nicht zu verhindern. Wir sind reaktiv und nicht proaktiv.

Software ist immateriell und daher ist es schwer zu verstehen, dass sie wie ein Auto gewartet werden muss. Kein vernünftiger Mensch würde ein Auto kaufen und es niemals warten. Wir sind uns darüber einig, dass eine solche Fahrlässigkeit die Lebensdauer verkürzt und langfristig die Kosten erhöht.

Trotz der Tatsache, dass viele Manager dies verstehen, werden sie für Änderungen am Produktionscode zur Verantwortung gezogen. Es gibt eine Politik, die es einfacher macht, gut genug in Ruhe zu lassen. Oftmals ist die Person, die überzeugt werden muss, über Ihrem Manager und er möchte einfach nicht kämpfen, um Ihre "großartigen Ideen" in die Produktion zu bringen.

Um ehrlich zu sein, ist Ihr Manager nicht immer davon überzeugt, dass Ihr Mittel tatsächlich so gut ist, wie Sie denken. (Schlangenöl?) Es gibt Verkäufergeist. Es ist Ihre Aufgabe, ihm zu helfen, den Nutzen zu erkennen.

Das Management teilt Ihre Prioritäten nicht. Setzen Sie Ihren Management-Hut auf und sehen Sie mit Management-Augen. Sie werden eher den richtigen Winkel finden. Hartnäckig sein. Sie werden mehr Veränderung bewirken, indem Sie nicht durch die erste negative Reaktion davon abgehalten werden.

Mario T. Lanza
quelle
1

Wenn Sie die angeforderte Änderung und den nicht angeforderten Refactor in zwei separate Änderungsanforderungen aufteilen können, wie von @ GlenH7 angegeben, ist dies das Beste, was Sie tun können. Sie sind dann nicht "der Mann, der Zeit verschwendet", sondern "der Mann, der doppelt so hart arbeitet".

Wenn Sie sich häufig in der Situation befinden, dass Sie sie nicht aufteilen können, weil für die angeforderte Arbeit jetzt der nicht angeforderte Refaktor zum Kompilieren erforderlich ist, würde ich vorschlagen, dass Sie auf Tools bestehen, mit denen Sie mithilfe der hier aufgeführten Argumente (dem Resharper) automatisch nach Codierungsstandards suchen können Warnungen allein können ein guter Kandidat sein, ich kenne mich damit nicht aus). Diese Argumente sind alle gültig, aber es gibt ein Extra, das Sie zu Ihrem Vorteil nutzen können: Wenn Sie diese Tests jetzt haben, ist es Ihre Pflicht , die Tests bei jedem Commit zu bestehen.

Wenn Ihr Unternehmen keine "Zeit für Umgestaltungen" verschwenden möchte (ein schlechtes Zeichen für sich), sollten Sie eine "Warnungsunterdrückungs" -Datei erhalten (jedes Tool verfügt über einen eigenen Mechanismus), damit Sie nicht mehr verärgert sind solche Warnungen. Ich sage dies, um Ihnen Optionen für verschiedene Szenarien zu bieten, auch für den schlimmsten Fall. Es ist auch besser, klare Vereinbarungen zwischen Ihnen und Ihrem Unternehmen zu treffen, auch über die erwartete Codequalität, als implizite Annahmen auf jeder Seite.

logc
quelle
Dies wäre ein guter Vorschlag für ein neues Projekt. Unsere aktuelle Codebasis ist jedoch riesig und Resharper gibt für die meisten Dateien viele Fehler aus. Es ist einfach zu spät, um es durchzusetzen, und das Unterdrücken vorhandener Fehler macht es noch schlimmer - Sie werden sie in Ihrem neuen Code vermissen. Außerdem gibt es viele Fehler, Probleme und Code-Gerüche, die ein statischer Analysator nicht erkennen kann. Ich habe lediglich Resharper-Warnungen als Beispiel gegeben. Auch hier hätte ich den "Verschwendungsteil" vielleicht etwas zu scharf formuliert, ich hätte etwas sagen sollen wie "Zeit für etwas, das keine Priorität hat".
t0x1n
@ t0x1n: Die Pfadfinder-Regel bezieht hauptsächlich die Module ein, die Sie berühren. Das kann Ihnen helfen, eine erste Trennlinie zu ziehen. Warnungen zu unterdrücken ist keine gute Idee, ich weiß, aber aus der Sicht des Unternehmens ist es richtig, sie in neuem Code zu unterdrücken und ihren Konventionen zu
folgen
1
Das ist der frustrierende Teil! Ich berühre nur Dateien, die ich als Teil meiner Aufgabe sowieso berührt hätte, aber ich erhalte trotzdem Beschwerden! Die Warnungen, über die ich spreche, sind keine Stilwarnungen, ich spreche über tatsächliche Leistungs- und Korrektheitsprobleme sowie über redundanten Code, doppelten Code usw.
t0x1n
@t0x1n: es klingt in der Tat sehr frustrierend. Bitte beachten Sie, dass ich damit nicht nur "statische Code-Analyse", sondern auch andere Empfehlungen gemeint habe, die Nexus entsprechen. Natürlich fängt kein Tool 100% ige Semantik ein. Dies ist nur eine Sanierungsstrategie.
Logc