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

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?

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.

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?".

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,

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.

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.

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.

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.

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.

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.

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.

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.

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.