Wie nähert man sich Entwicklern, die an Code-Qualitätsstandards arbeiten?

Ich habe die Verantwortung für eine Gruppe von 3 Entwicklern mit schrecklicher Codequalität in ihrem Projekt übernommen. Um die Codequalität zu erhöhen, wurden viele Meetings abgehalten und CI um eine Codequalitätskontrolle (Sonarqube) erweitert, um den Code zu überwachen und die Pipeline zum Scheitern zu bringen, wenn sie die Anforderungen nicht erfüllt.

Einer der Entwickler hat einen Weg gefunden, um Funktionskomplexitätsgrenzen zu umgehen und fehlerhaften Code zu übertragen (Beispiel unten). Meine Frage ist, wie ich das angehen soll, um zu verhindern, dass er und andere Entwickler Problemumgehungen verwenden, anstatt nachzudenken und ihre Codes zu reparieren.

switch (true) { 
               case (first & second & otherthing):
                    dosomething();
                    break;
               case (unrelated_if || complex):
                    do_totally_unrelated_thing_to_previous_one();
                    break;
               ...
               }
Wenn Sie genug Zeit haben, können Sie sie zwingen, nur Pull-Requests zu senden, dann könnten Sie diese überprüfen und solche bs einfach ablehnen.
Ich stimme dem obigen Kommentar zu. Dass sie Anfragen stellen müssen, bedeutet auch, dass Sie, wenn Sie bs im Code oder Code finden, der nicht den Standards entspricht, den Code leicht überprüfen können, um sicherzustellen, dass dies nicht der Fall ist.
Haben Sie diese Typen jemals gefragt, warum sie Code von schlechter Qualität schreiben? Und wenn ja, was war ihre Antwort?
@FlorianSchaetz Wir haben Gitlab und alle Codes werden nur zusammengeführt, indem eine Zusammenführungsanfrage (Pull) gestellt wird, die von mir geprüft wird. Das Problem ist, dass es so viel Zeit in Anspruch nimmt, jede einzelne Zusammenführungsanforderung zu schließen, sie umzuschreiben und erneut zu testen. Ich möchte sie dazu bringen, weniger bs in Merge-Anfragen zu schreiben
Können Sie boolesche Literale einschalten? Das wusste ich gar nicht. Sie versuchen, den Codeüberprüfungsprozess zu automatisieren, was nur funktioniert, wenn die Entwickler wissen, wie guter Code aussieht. Dies scheint bei Ihren Junior-Entwicklern nicht der Fall zu sein, also müssen Sie zuerst darin investieren. Das Lesen über spartanische Programmierung hat mir geholfen, Klarheit in meinen Code zu bringen, vielleicht hilft es auch anderen.
@rath: Das kannst du. In C oder C++ bedeutet dies, dass Sie nur Konstanten true und false als Fälle haben können. In anderen Sprachen findet dies den ersten Fall, der als wahr ausgewertet wird, oder fährt mit dem Standard fort. In noch anderen Sprachen kann es jeden Fall ausführen, der als wahr ausgewertet wird. Aber klar, das ist nicht das, was der Entwickler wollte, sondern ein Hack, um das automatische Tool zu umgehen.
Gibt es keine Regeln auf Sonarqube, die Sie aktivieren können, um sie zu verbieten switch(true)? Oder Sie könnten Ihre eigene Regel schreiben, um sie hinzuzufügen. Wenn sie etwas getan haben, das sie nicht richtig schreiben können, sollten Sie ihnen das erklären, wenn sie sich überhaupt für Codequalität interessieren ...
Haben Sie Codierungsstandards eingeführt? Wer entscheidet, was „schreckliche Codequalität“ ist?
Die einfache Antwort auf Ihre Frage ist "Feuer sie". Es ist letztendlich der einzige Weg, jemanden dazu zu bringen, etwas zu tun. Sagen Sie einfach: "Ich habe schon oft erwähnt, dass die Codequalität niedrig ist. Dies ist eine letzte Warnung, und dann müssen wir einen oder alle drei von Ihnen gehen lassen." Beachten Sie, dass in fast jeder Gruppe von etwa drei Programmierern etwa alle sechs Monate etwa einer wegen Inkompetenz gefeuert wird . Was ist die große Sache hier? Feuern Sie einfach das Schlimmste.
Tut mir leid, hier der Idiot zu sein, aber was ist falsch an diesem Code? Wenn Sie zwischen komplexen Bedingungen wechseln müssen, ist diese Methode effizient und ermöglicht einen Code, der der Spezifikation folgt.
@ gazzz0x2z im Allgemeinen möchten Sie keine Codestruktur, die 2 unabhängige Funktionen verarbeitet. Das Programm sollte sich auf eine einzelne Aufgabe mit einem anderen Programm/Programmierstruktur für jede einzelne Aufgabe konzentrieren. Aus der Debug-Perspektive ermöglicht dies das Eingrenzen und Korrigieren von Fehlern viel einfacher, da Sie keine miteinander verbundenen Aufgabenregionen haben.
Schlechtes Werkzeug. Stattdessen Peer-Review einführen. Die Investition in Zeit zahlt sich erheblich aus, wenn die Wartung beginnt.
@ gazzz0x2z Die Codestruktur ist fast definitiv ein zu ersetzender Hack, if(x){} if(y){}da der statische Analysator die ifAnweisungen für die Qualitätskontrolle markiert hat, aber nicht die switch. Ob es ohne diesen Kontext in Ordnung ist oder nicht, ist nicht wirklich wichtig.
Wurde die enthaltende Funktion im Commit neu erstellt oder war sie ein Fix für eine bereits vorhandene Funktion?

Antworten (4)

Persönlich finde ich die meisten dieser automatisierten Code-Tools nutzlos. Es gibt Zeiten, in denen es fehlschlägt, Dinge zu programmieren, die einfach Vorlieben sind, und Dinge, die unter bestimmten Umständen schlecht, aber unter anderen gut oder sogar notwendig sind. Und oft lassen sie den Entwickler unsicher, was die eigentliche Lösung sein sollte. Wenn Sie wissen, dass etwas fehlschlägt, aber nicht verstehen, warum es fehlschlägt oder was Sie stattdessen tun sollten, ist das Tool selbst fehlgeschlagen.

Was hilft, ist 100%iger Code-Review. Kein Code wird an den Produktionszweig übergeben, ohne durch eine Codeüberprüfung akzeptiert zu werden, und kein Entwickler hat das Recht, nur das Build-Team oder den Lead an den Produktionszweig zu übertragen.

Hier senden Sie den schlechten Code zurück, vorzugsweise mit einer Erklärung, warum er schlecht ist. Der Schlüssel ist, es schmerzhaft zu machen, den Code nicht zu reparieren. Ja, sie werden einige Male die Frist versäumen, weil der Code die Codeüberprüfung nicht bestanden hat. Und das müssen sie als Grund erklären. Dies führt dazu, dass Menschen mit geringerer Wahrscheinlichkeit denselben Fehler wiederholt machen, damit sie ihre Fristen einhalten können. Wenn es keine Schmerzen bereitet, schlechten Code zu schreiben, gibt es keinen Grund, ihn zu reparieren, da die menschliche Natur so ist, wie sie ist.

Allerdings müssen Sie und Ihr Team sich darüber einigen, was guter Code und was akzeptabler Code ist. Wenn Ihre Standards und ihre derzeit nicht übereinstimmen, muss dies in Gesprächen gelöst und ein akzeptabler Standard genehmigt werden. Wenn sie Beiträge zum Standard geleistet haben (und ja, das bedeutet, dass Sie ihre Standards zumindest teilweise kompromittieren und akzeptieren müssen, die Diskussion ist irrelevant, sogar kontraproduktiv, wenn Sie immer noch Endergebnisse diktieren), wird dies der Fall sein mehr kaufen, um es tatsächlich zu benutzen.

Es gibt Zeiten... Natürlich gibt es sie. Aus diesem Grund sind die meisten Tools konfigurierbar . Sie können beispielsweise Regeln wechseln, Regelprioritäten ändern oder sogar neue Regeln hinzufügen. Statische Codeanalysatoren können ein wertvolles Werkzeug sein, das viele typische Probleme aufzeigt, aber sie sind nur der Anfang von gutem Code und nicht das Ende. Es kann einige grundlegende Aufgaben erledigen, aber für mehr benötigen Sie natürlich immer Code-Reviews.
Ich werde mich für einige Code-Tools aussprechen. Resharper ist mein Favorit. Sie (HLGEM) leiden möglicherweise unter dem „Fluch des Wissens“ – Sobald Sie anfangen, wohlgeformten Code automatisch zu schreiben, sehen Sie den Wert des Tools nicht mehr. Das Tool kann jedoch von unschätzbarem Wert sein, um einen Entwickler dazu zu bringen, wohlgeformten Code zu schreiben. Meine Meinung. YMMV. +1 für den Rest des Inhalts.
Vielleicht liegt es daran, dass die einzigen, die ich verwendet habe, für SQL-Code waren und sie allgemein schrecklich sind, weil sie die Bedeutung nicht berücksichtigen und die Leute denken, dass ihr Code gut ist, weil er bestanden wurde, wenn die Ergebnisse falsch waren und wir die Regel umgehen mussten für Aus Leistungsgründen bedurfte es praktisch eines Kongressbeschlusses. Außerdem habe ich viel zu viele Leute gesehen, die völlig verwirrt darüber waren, womit die korrelierte Unterabfrage ersetzt werden sollte, oder nicht in der Lage waren, eine Änderung vorzunehmen und dieselbe Ergebnismenge zu erhalten.
Meine Erfahrung mit Menschen, die diese Tools verwenden, ist außerdem, dass sie weniger nachdenken müssen, was es ihnen erschwert, auf ein fortgeschrittenes Niveau zu gelangen, und dass sie Dinge auswendig tun, weil das Tool sagt, dass sie schlecht sind, und nicht, weil sie verstehen, warum sie schlecht sind oder welche Bedingungen die Regel benötigen könnte, um eine Ausnahme zu haben. Menschen weniger analytisch zu machen, ist schlecht für den Berufsstand.
@FlorianSchaetz Es gibt einen Unterschied zwischen dem Ausführen eines statischen Analysetools zum Suchen nach möglichen Fehlern (eine gute Idee) und dem Setzen eines Eincheckblocks basierend auf seiner Ausgabe (eine schlechte Idee). Die statische Analyse kann einige Fehler finden, sie wird auch eine Menge falsch positiver Ergebnisse und bedingte Probleme hervorrufen, bei denen sie einfach nicht schlau genug ist, um zu wissen, ob etwas in Ordnung ist. Sie führen auch sinnlose Nits auf und machen große Geschäfte aus kleinen Stilunterschieden. Führen Sie sie aus und sehen Sie sich die Ergebnisse an, aber die einzige Möglichkeit, die Qualität sicherzustellen, ist eine tatsächliche menschliche Überprüfung, nicht eine automatisierte.
Wenn andererseits nichts die Ergebnisse des Analysators erzwingt, wird es ignoriert. Persönlich neige ich dazu, eine Mischung daraus zu haben, sie nicht für jeden Pull-Request durchzusetzen, sondern alle Probleme während der letzten Refactoring-Phase durchzuarbeiten.
Es sind nicht nur ihre inhärenten technischen Einschränkungen, die diese Tools problematisch machen – Menschen können ihr Ziel und ihre Verantwortung darin sehen, das Tool zufrieden zu stellen, anstatt das zugrunde liegende Ziel zu erreichen. Es zu einem starken administrativen Hindernis zu machen, wird die Aufmerksamkeit nur noch mehr auf das Tool lenken. Das Tool sagt ja, also sind wir fertig, richtig? Eine falsch durchgeführte Codeüberprüfung riskiert, nur ein weiteres mächtigeres Werkzeug mit demselben Problem zu sein, gut gemacht (100 % ist vielleicht zu viel) ist es eine Möglichkeit, Leute dazu zu bringen, miteinander zu reden und über die Codequalität nachzudenken. Finden Sie Wege, um die Kultur zu verändern, anstatt Details zu verwalten.

Sie haben ein Tool eingeführt, das Ihnen anscheinend nur im Weg steht. Der schreckliche Code, den Sie gepostet haben, wurde erstellt, weil der Entwickler ursprünglich Code erstellt hat, der von Ihrem Tool nicht akzeptiert wurde, und herausgefunden hat, wie er durch Verschlechtern des Codes akzeptiert werden würde. Das ist ganz dein Problem. Wenn Sie Situationen schaffen, in denen Menschen für das Falsche belohnt werden, werden sie das Falsche tun.

Was wir nicht wissen, wenn wir nur eine Seite der Geschichte hören, ist, ob sie eine schreckliche Codequalität haben oder ob sie Code haben, den Sie nicht mögen – was eine ganz andere Sache sein kann. Sie sind ein erfahrener Entwickler? Sagen Sie ihnen dann, wie sie den Code verbessern können, schicken Sie sie zu Schulungen und führen Sie Code-Reviews durch. Oder sind Sie ein spitzhaariger Chef? Lassen Sie sie in diesem Fall weitermachen.

Ich habe fünf oder sechs Jahre Entwicklungserfahrung, während sie etwa zwei Jahre haben.
In diesem Fall schulen und ihnen sagen, wie sie ihren Code verbessern können.
Training sollte Hand in Hand gehen mit der Umsetzung von Regeln wie diesen – Sie haben einen Weg zum Scheitern geschaffen, stellen Sie sicher, dass Sie ihnen auch einen Weg zum Erfolg geben.
+1 Menschen tun, wozu sie motiviert sind. Richten Sie das System und die Prozesse so ein, dass sie die Anreize haben, Ihre Ziele zu erreichen.
Ich stimme zwar zu, dass dies nur eine Seite der Geschichte ist (obwohl welche Frage hier keine ist?), aber diese Seite (zusammen mit diesem Code) lässt es wie nichts als Ungehorsam erscheinen, was nur durch Training oder Anleitung behoben werden kann, wenn die Insubordination ist das Ergebnis unklarer Regeln. Wenn sie aufsässig sind, einfach weil sie die Autorität von OP nicht respektieren oder weil sie anderer Meinung sind (was sehr viel wahrscheinlicher erscheint, da zwei beliebige Entwickler eine 99% ige Chance haben, sich über Codierungsstandards nicht einig zu sein), werden Schulung und Anleitung nichts bewirken.

Hier gibt es zwei Probleme:

  • ihre Codequalität ist schlecht

  • Sie arbeiten um Ihren Code Quality Enforcer herum

Es gibt eine einfache Lösung: Code-Review.

Überprüfen Sie jeden Pull-Request, den sie stellen. Wenn sie Code von schlechter Qualität begehen, erklären Sie, warum es schlechte Qualität ist. Erklären Sie, warum Qualitätsstandards wichtig sind. Erklären Sie, dass bestimmte Designentscheidungen kurzfristig schneller sein können, aber erhebliche technische Schulden mit sich bringen. Erklären Sie, dass es nicht akzeptabel ist, absichtlich Workarounds an Ihren Coding Quality Enforcer zu schreiben. Der Schlüssel hier ist, ihnen beizubringen, warum es wichtig ist, und ihnen nicht nur zu sagen, was sie tun sollen. Akzeptieren Sie die Pull-Requests erst, wenn alle erforderlichen Änderungen vorgenommen wurden.

Wenn sie nach ein paar Runden immer noch schlechten Code schreiben und Workarounds verwenden, kann dies ein Zeichen von Inkompetenz oder Insubordination sein, die Sie angemessen ansprechen sollten. Aller Wahrscheinlichkeit nach sind sie es nicht gewohnt, Code in einem neuen Stil zu schreiben, und brauchen einige Zeit, um sich anzupassen. Es ist Ihre Aufgabe als Betreuer, ihnen beim Lernen und Anpassen zu helfen, aber wie heißt es so schön: Sie können ein Pferd zum Wasser führen, aber Sie können es nicht zum Trinken bringen.

Wenn sie sich weigern, die ihnen gegebenen Regeln zu befolgen, ist es einfach. Geben Sie ihnen eine Warnung, sagen Sie in dieser Warnung, dass es Konsequenzen geben wird, wenn sie 2 Warnungen erhalten. Die Tatsache, dass Sie für sie verantwortlich sind, bedeutet, dass die Konsequenzen für Sie gelten, wenn sie dies weiterhin tun.

Gehen Sie auf Nummer sicher, machen Sie schriftliche (per E-Mail) Regeln darüber, was sie tun MÜSSEN. Wenn sie diese Regeln nicht befolgen, melden Sie dies Ihrem Vorgesetzten.

Sprich auch mit ihm, es könnte etwas nicht stimmen. Das Schreiben von schlechtem Code könnte daran liegen, dass es ein Problem in seinem beruflichen/privaten Bereich gibt. Stellen Sie also sicher, dass das nicht die Sache ist, die ihn dazu bringt, schlechten Code zu begehen.

-1 Dies ist ein sehr hartnäckiger Ansatz. Wenn ich der Manager des OP wäre, wäre meine erste Frage, funktioniert der Code? Dann würde ich erwarten, dass OP argumentiert, dass die zukünftige Fehlerbehebungszeit durch die aktuelle Codequalität erheblich verlängert wird, und dann würde ich in die Schulung der Junior-Entwickler investieren. Es gibt nichts auf Nummer sicher zu gehen, weil es nichts zu spielen gibt. Dies ist alles in allem ein kleines Problem
Wenn jemand anderes das Projekt mit schlecht geschriebenem Code übernehmen muss, wird es viel Zeit, Mühe und Geld kosten, um die Probleme zu beheben. Nicht in der Lage zu sein, Code zu schreiben, der die Anforderungen erfüllt, bedeutet, dass Sie Ihre Arbeit nicht richtig machen. Das Aufstellen von Regeln stellt sicher, dass die Arbeitnehmer die Möglichkeit haben, die Dinge für sich und das Unternehmen zu verbessern und nicht zu verschlechtern. @rat
@FlorianSchaetz In einer Ideenwelt ja, aber leider leben wir nicht dort. Gut geschriebener Code ist manchmal ein Luxus. Arbeitscode hält die Lichter an. Es liegt an uns, die Balance zu finden. Es ist nicht ratsam, ein Feature oder eine Fehlerbehebung zurückzuhalten, weil Sonarqube dies gesagt hat. Kein Tool sollte dem Geschäft im Wege stehen. Für all das gilt natürlich YMMV.
Sauberen Code zu schreiben, der den Standards entspricht, ist kein Privileg. Wenn Sie wissen, dass Ihr Code nicht gut genug ist, sollten Sie etwas daran ändern. Wenn sich die Kollegen anstrengen, ist das eine ganz andere Geschichte. OP erwähnte, dass einer der Entwickler „einen Weg gefunden hat, um Funktionskomplexitätsgrenzen zu umgehen“. Das bedeutet, dass sie sich mehr Mühe geben, das Problem zu umgehen, als daran zu arbeiten, es zu beheben
Nein, tut mir leid, gut geschriebener Kabeljau ist niemals ein Luxus. Schlecht geschriebener Code bedeutet, aktiv auf eine Katastrophe hinzuarbeiten, denn früher oder später werden Sie einen Punkt erreichen, an dem Sie ihn nicht mehr warten können und an dem es unmöglich ist, die Fehler zu beheben oder neue Funktionen hinzuzufügen, ohne vorhandenes Material zu beschädigen. Es geht nicht um Tools, sondern um sauberen Code. Und solange Sie keine Software haben, die Sie nicht warten müssen (Fire-and-Forget-Software), leiht sich schlechter Code nur Zeit aus einer ungewissen Zukunft.
@rath Du solltest wirklich Software Engineering studieren. Seit den 80er Jahren ist bekannt, dass Ihr Ansatz zu weniger wartbarer, weniger zuverlässiger und teurerer Software führt. Sie verlieren das Firmengeld, indem Sie Mist-Software schreiben. Jetzt können wir darüber streiten, was die Definition von Mist ist, aber das OPs-Beispiel ist definitiv qualifiziert.
@GabeSechan Ich stimme dir vollkommen zu. Manchmal fangen Sie mit schlechtem Code an, und jede Anstrengung, ihn in Form zu bringen, wird am Ende mehr Zeit verschwenden, als Sie sich leisten können. Es gibt Code-Zen und es gibt absolute WTF-Ery, und irgendwo dazwischen gibt es die Goldilock-Zone von halb beschissen, aber einigermaßen brauchbar. Jeder liest das Obige und stellt es in seinen Kontext. Ich arbeite mit Legacy-Code und stehe jeden Tag vor diesem Dilemma.