Wie kann ich meine Teamkollegen davon überzeugen, dass wir Compiler-Warnungen nicht ignorieren sollten?

51

Ich arbeite mit Eclipse in Java an einem riesigen Projekt (eher an einer verwickelten Kombination von Dutzenden von Miniprojekten, die sich aufgrund eines schlechten Abhängigkeitsmanagements nicht leicht trennen lassen, aber das ist eine andere Diskussion). Wir haben bereits eine Reihe von Warnungen in den Compilereinstellungen deaktiviert und das Projekt enthält immer noch über 10.000 Warnungen.

Ich bin ein großer Befürworter des Versuchs, alle Warnungen anzugehen, wenn möglich zu beheben, und für diejenigen, die untersucht werden und als sicher gelten, sie zu unterdrücken. (Gleiches gilt für meine religiöse Besessenheit, alle implementierten / überschriebenen Methoden als @Override zu markieren). Mein größtes Argument ist, dass generell Warnungen Ihnen helfen, potenzielle Fehler während der Kompilierungszeit zu finden. Möglicherweise sind die Warnungen 99 von 100 Mal unbedeutend, aber ich denke, dass das Kratzen des Kopfes, das es zum einen spart, dass es einen größeren Fehler verhindert, das alles wert ist. (Mein anderer Grund ist meine offensichtliche OCD mit Code-Sauberkeit).

Viele meiner Teamkollegen scheinen sich jedoch nicht darum zu kümmern. Ich behebe gelegentlich Warnungen, wenn ich auf sie stoße (aber Sie wissen, dass es schwierig ist, von einem Kollegen geschriebenen Code zu berühren). Jetzt mit buchstäblich mehr Warnungen als Klassen, werden die Vorteile von Warnungen sehr stark minimiert, weil sich niemand die Mühe machen wird, sie alle zu untersuchen, wenn Warnungen so alltäglich sind.

Wie kann ich meine Teamkollegen (oder die vorhandenen Kräfte) davon überzeugen, dass Warnungen angesprochen (oder unterdrückt) werden müssen, wenn sie vollständig untersucht wurden? Oder soll ich mich davon überzeugen, dass ich verrückt bin?

Vielen Dank

(PS: Ich habe vergessen zu erwähnen, was mich letztendlich dazu veranlasst hat, diese Frage zu stellen: Ich habe leider bemerkt, dass ich Warnungen langsamer behebe, als sie produziert werden.)


quelle
5
Alle Arten: Rohtyp, nicht verwendeter Import, nicht verwendete Variable, nicht verwendete private Methoden, unnötige Unterdrückung, nicht aktivierte, unnötige Umwandlung, unnötige Bedingung (immer wahr oder falsch), nicht kommentierte überschriebene Methoden, Verweis auf veraltete Klasse / Methoden usw.
1
«Ich möchte damit beginnen, statische Code-Analysen auf Spielkarten anzuwenden und Warnungen als Fehler zu behandeln. »Aktuelles Zitat von John Carmack. Wenn Sie verrückt sind, sind Sie zu Ihnen. Eigentlich sind wir drei von uns.
Deadalnix
1
@RAY: Dies sind Warnungen aus der Eclipse-Codeanalyse. Ich bin mir nicht sicher, ob Sie alle von Vanille beziehen können javac.
Yatima2975
4
Aber Sie wissen, dass es schwierig ist, von einem Kollegen geschriebenen Code zu berühren. Wenn Ihr Arbeitsplatz solche Probleme mit der Territorialität hat, halte ich das für eine große rote Fahne.
JSB ձոգչ
1
@deadalnix: Da ich ein C ++ - Typ bin, habe ich nur eine Möglichkeit, Dinge zu tun -Wall -Wextra -Werror(dh die meisten verfügbaren Warnungen aktivieren, alle als Fehler behandeln). Eclipse C ++ ist jedoch nahezu unbrauchbar: /
Matthieu M.

Antworten:

37

Sie können zwei Dinge tun.

  1. Weisen Sie darauf hin, dass Warnungen aus einem bestimmten Grund vorhanden sind. Compiler-Autoren geben sie nicht ein, weil sie gemein sind. Menschen in unserer Branche sind im Allgemeinen hilfreich. Viele Warnungen sind hilfreich.

  2. Sammeln Sie eine Geschichte spektakulärer Fehler, die sich aus ignorierten Warnungen ergeben. Eine Websuche nach "Compiler-Warnungen beachten" liefert einige Anekdoten.

Ihre Besessenheit mit @Overrideist keine "Besessenheit". Das ist eine gute Sache. Haben Sie jemals einen Methodennamen falsch geschrieben?

Ray Toal
quelle
8
Ich möchte hinzufügen, dass Sie andere nicht dazu bringen können, sich darum zu kümmern. Seien Sie also pragmatisch und wählen Sie Ihre Schlachten gut aus.
Daniel Werner
@ Daniel: traurig, aber wahr. Wenn es ihnen egal ist, auch wenn sie wissen (oder zumindest glauben ), dass Sie Recht haben, dann ist dies keine Aufgabe mit rosigen Perspektiven.
Joachim Sauer
2
Es gibt viele Male, in denen Warnungen eigentlich nur Warnungen sind. Sie können wachsam sein, aber ich würde es oft vorziehen, diese Zeit damit zu verbringen, Testfälle zu schreiben, die viel spezifischer für Ihre Codebasis sind.
Ghayes
Ich denke, die OP versucht, die Mitarbeiter dazu zu bringen, nicht länger abzulehnen. Zugegeben, viele Warnungen müssen nicht angesprochen werden und sollten es auch nicht alle sein! Aber es ist keine gute Sache, blasiert zu sein und 10.000 von ihnen in die Codebasis einzuschleichen. Warnungen sollten geprüft und berücksichtigt werden. Wenn sie nicht zutreffen, können sie deaktiviert oder eine Unterdrückungsannotation angewendet werden.
3
Ich habe kürzlich einige Warnungen in einem Open Source-Projekt aufgeräumt und einen klaren Fehler gefunden, der über eine Warnung gemeldet wurde: if (error = 0)statt if (error == 0). Außerdem erleichtern viele Warnungen das Auffinden von Compilerfehlern , ohne sich durch unzählige Warnungen wühlen zu müssen.
Hugo
12

Relevante Lektüre hier . C ++, aber immer noch relevant. Ich mag dieses Beispiel besonders (Code-Kommentar ist meins):

int searchArray(int to_search[], int len, int to_find)
{
    int i;
    for( i = 0; i < len; ++i )
    {       
        if ( to_search[i] == to_find )
        {
            return i;
        }
    }
    // should be returning designated 'not found' value (0, -1, etc)
}

In den meisten Fällen bedeutet eine Warnung, dass die Anwendung mit einem Fehler abstürzt. Dies hilft Ihnen dabei, einen Konstruktionsfehler aufzuspüren und zu beheben (z. B. sicheres Typumwandeln), oder dass die Anwendung Annahmen trifft und sich dann tatsächlich falsch verhält oder auf falsche Weise (was bei unsicherer Typumwandlung der Fall wäre ). In ähnlicher Weise weisen sie auch auf Cruft- oder Dead-Code hin, der entfernt werden kann (nicht erreichbare Codeblöcke, nicht verwendete Variablen), der als Codeoptimierung betrachtet werden kann, oder auf unberücksichtigte Fälle, die beim Testen auftreten, wie im obigen Beispiel für C ++. Beachten Sie, dass dieses Beispiel zu einem Fehler bei der Kompilierung in Java führt.

Das Beheben von Warnungen hat (mindestens) mehrere Vorteile für das Management:

  1. Weniger Chancen, dass sich der Code bei vom Benutzer eingegebenen Daten nicht richtig verhält, was für die Betroffenen, die sich um das Image des Unternehmens und den Umgang mit den Daten seiner Kunden sorgen, eine große Sache sein sollte. Abstürze, um einen Benutzer daran zu hindern, etwas Dummes zu tun, sind besser, als nicht zusammenzustürzen und es zuzulassen.
  2. Wenn sie Personal neu mischen möchten, können die Leute in eine warnfreie Codebasis geraten (und nicht so schnell ausflippen oder sich beschweren ..). Vielleicht wird dadurch das Murren oder die Meinung über die Wartbarkeit oder Qualität des Beitrags von Team X zu Projekt Y verringert.
  3. Durch die Optimierung des Codes durch Entfernen von Compiler-Warnungen wird die Laufzeit zwar nicht wesentlich verbessert, aber beim Testen werden zahlreiche Punkte erzielt:
    • Jeder Code-Coverage-Score wird sich wahrscheinlich erhöhen.
    • Testgrenzen und ungültige Testfälle werden wahrscheinlich häufiger erfolgreich sein ( unter anderem aufgrund der oben genannten sicheren Typumwandlung).
    • Wenn ein Testfall fehlschlägt, müssen Sie nicht darüber nachdenken, ob er auf eine der Compiler-Warnungen in dieser Quelldatei zurückzuführen ist.
    • Es ist leicht zu argumentieren, dass Null-Compiler-Warnungen besser sind als Tausende. Keine Warnungen oder Fehler sprechen für die Qualität des Codes. Sie können also argumentieren, dass die Codequalität umso besser ist, je weniger Warnungen vorhanden sind.
  4. Wenn Sie keine Compiler-Warnungen haben und eine oder mehrere eingeführt wurden, ist es viel einfacher zu wissen, wer diese Warnungen in der Codebasis verursacht hat. Wenn Sie eine Richtlinie "Keine Warnungen" haben, hilft das ihnen, Leute zu identifizieren, die ihre Arbeit nicht richtig machen, vielleicht? Beim Schreiben von Code geht es nicht nur darum, dass etwas funktioniert, sondern auch darum, dass es gut funktioniert .

Beachten Sie, dass ich immer noch ein Junior-Entwickler bin, der nur wenig über Projektmanagement weiß. Wenn ich also etwas Falsches gesagt habe, korrigieren Sie bitte mein Denken und geben Sie mir die Möglichkeit, es zu bearbeiten, bevor Sie mich aus dem Leben werfen :)

darvids0n
quelle
2
sehr gut durchdachte antwort. Danke. Das Beispiel, das Sie angeben, ist in Java jedoch eher ein Fehler als eine Warnung. :)
@ Ray: Ah, fair genug. Es ist ein Jahr her, dass ich Java-Entwickler gemacht habe, also musste ich etwas übersehen. Ich werde meinen Beitrag bearbeiten, um das zu erwähnen. Vielen Dank!
Darvids0n
Es ist schon eine Weile her, dass ich C ++ gemacht habe. Was gibt diese Funktion zurück, wenn Sie den Kommentar am Ende erreichen? Ist es 0? Undefiniertes Verhalten?
MatrixFrog
1
@ MatrixFrog: Undefiniert. Kann ein beliebiger Int sein, der alle möglichen Unangenehmen hervorruft. Zitat aus dieser Antwort: " C ++ 03 §6.6.3 / 2: Das Abfließen einer Funktion entspricht einer Rückgabe ohne Wert; dies führt zu undefiniertem Verhalten in einer Wert-Rückgabefunktion. "
darvids0n
7

Ich habe in der Vergangenheit an einem Projekt mit ähnlichen Eigenschaften gearbeitet. Insbesondere dieses wurde ursprünglich in Java 1.4 geschrieben. Sobald Java 5 mit Generika herauskommt, kann man sich die Anzahl der Warnungen vorstellen, die der Compiler für jede Verwendung der Collections-API ausgibt.

Es hätte einige Zeit gedauert, bis alle Warnungen beseitigt waren und Generika zum Einsatz kamen. Dies ist ein Faktor, der berücksichtigt werden muss. Wenn Sie jedoch jemanden (insbesondere Ihren Vorgesetzten) davon überzeugen müssen, dass er repariert werden muss, benötigen Sie beispielsweise harte Daten

Die Zeit, die für Fehler in veraltetem oder nicht standardkonformem Code aufgewendet wurde (der Standard ist der Codierungsstandard Ihres Projekts), die vermieden werden konnten.

Sie könnten weiterhin eine Rechnung vorlegen, die zeigt, wie Sie Zeit und Geld verlieren, indem Sie "bestimmte" Warnungen ignorieren, und jemand wird auf die Idee kommen. Der Schlüssel ist, dass nicht alle Warnungen einen Blick wert sind, zumindest nicht auf Anhieb. In einfacheren Worten, Sie müssen die Priorität der Warnungen festlegen, die sofort angesprochen werden müssen. Betrachten Sie zunächst diejenigen, die für Fehler in Ihrem Projekt relevant sind.

Wenn Sie Fehler beheben, können Sie im Bug-Tracker auch Notizen machen, dass dieser Fehler möglicherweise vermieden wurde, indem eine Warnung nicht ignoriert wurde (oder indem PMD oder FindBugs ausgeführt wurden, wenn Sie ein CI oder ein Build-System haben). Solange es eine ausreichende Anzahl von Fehlern gibt, die durch das Beachten von Compiler-Warnungen behoben werden können, ist Ihre Überlegung, sich Compiler-Warnungen anzusehen, gültig. Ansonsten ist es eine geschäftliche Entscheidung, und normalerweise lohnt es sich nicht, Zeit für diesen Kampf zu investieren.

Vineet Reynolds
quelle
Ja genau. Ich denke, die anfängliche Explosion der Anzahl der Ausnahmen geschah während des 1.4-> 1.5-Übergangs, und seitdem hat niemand mehr auf Warnungen geachtet, weil es einfach zu viele gibt ...
2

Wenn Sie erfolgreich sind und Ihr Team die Warnungen beachtet, sollten Sie schrittweise vorgehen. Niemand kann und wird auf einmal 10000 Warnungen auslösen. Sie können also die wichtigsten auswählen (Fehler mit hoher Wahrscheinlichkeit) und die weniger wichtigen deaktivieren. Wenn sie behoben sind, erhöhen Sie die Warnstufe erneut. Außerdem können Sie mit FindBugs beginnen, das vor fast immer fehlerhaftem Code warnt.


quelle
2
Oder konzentrieren Sie sich auf das Beheben von Warnungen, wenn Sie sich den Code ansehen (neue Klassen sollten keine Warnungen enthalten, geänderte Klassen sollten weniger Warnungen enthalten).
Setzen Sie Monica
Ernste Frage: Woher wissen Sie, welche Fehler am wahrscheinlichsten sind?
MatrixFrog
1

Ich stimme Ihnen voll und ganz zu. Es wird empfohlen, die Kompilierungswarnungen so weit wie möglich zu bereinigen. Sie haben erwähnt, dass Ihr Team Eclipse als Entwicklungswerkzeug verwendet. Eclipse ist ein sehr gutes Tool, mit dem Sie den Code bereinigen und die Konsistenz des Codestils sicherstellen können.

  1. Definieren Sie für jedes Java-Projekt die Option "Aktion speichern", z. B. Formatierungscode, nicht verwendete lokale Variablen, Organisation der Importe (ich glaube, niemand hat Einwände gegen eine solche Bereinigung).
  2. Definieren Sie projektspezifische Kompilierungsoptionen für jedes Projekt. Beispiel: Kompilierungsstufe (wenn Ihr Code mit 1.4 kompatibel sein muss, um die Warnungen in Bezug auf generic zu bereinigen), hat die Zuweisung keine Auswirkung (z. B. 'x = x') und so weiter. Sie und Ihr Teamkollege können eine Vereinbarung für diese Elemente treffen, und Eclipse meldet sie in Ihrer Entwicklungsphase als "Fehler", nachdem Sie sich auf den Kompilierungsfehler geeinigt haben.

Eclipse erstellt einige Eigenschaftendateien für diese Einstellungen im Ordner .settings. Sie können sie in andere Java-Projekte kopieren und als Teil des Quellcodes in SCM einchecken.

Sie können den Code von Eclipse überprüfen, um zu sehen, wie Eclipse-Entwickler dies tun.

Kane
quelle
1

Sie haben zwei Möglichkeiten, um alle Warnungen loszuwerden (und ich bin damit einverstanden, dass ein subtiler Fehler verborgen bleibt):

  1. Überzeugen Sie das gesamte Team, dass dies das Beste ist, und lassen Sie es beheben.
  2. Überzeugen Sie Ihren Chef, dass dies das Beste ist, und machen Sie es verbindlich. Dann müssen sie es reparieren.

Ich glaube, dass 1 in Ihrem Fall nicht mehr machbar ist. Auch dies wird aufgrund der Anzahl der Warnungen, die in der Produktivitätsausgabe angezeigt werden, wahrscheinlich so lange dauern, dass das Management dies ohnehin wissen muss.

Also, mein Vorschlag ist: Nehmen Sie es mit dem Chef auf, überzeugen Sie ihn, dass dies eine tickende Bombe ist, und lassen Sie es offizielle Richtlinien festlegen.

Thorbjørn Ravn Andersen
quelle
1

Ich würde ihnen nur sagen, dass die meisten davon unbedeutende Warnungen sind und es wichtig ist, dass wir sie von der Liste streichen. Damit wir die wirklich bedeutende Warnung in der Menge nicht verpassen, wie es passiert!

WinW
quelle
Genau. Es ist wichtig, sie loszuwerden, weil sie unbedeutend sind.
MatrixFrog
0

Sie brauchen viel Geduld, um sie zu überzeugen, und das Entfernen von Warnungen beim Pair-Programming hilft anderen, die Gewohnheit wieder aufzunehmen. Schlagen Sie ihnen vor, Effective Java 'Punkt 24: Deaktivieren Sie ungeprüfte Warnungen' (für die generische Sammlung) zu lesen. Und ignoriere wahrscheinlich viele Fälle, in denen Leute deinem Rat nicht folgen;)

Mehul Lalan
quelle
0

Ein effektiver Ansatz wäre hier, einen Continuous Integration- Server einzurichten, der das Projekt automatisch erstellt und die Tests immer dann ausführt, wenn jemand Code eincheckt. Wenn Sie dies nicht bereits nutzen, sollten Sie es tun, und es ist nicht sehr schwer, andere von den Vorteilen davon zu überzeugen.

  1. Überzeugen Sie Management- / Teamleiter von den Vorteilen dieser Vorgehensweise (Verhindern der Bereitstellung fehlerhafter Builds, frühzeitiges Auffinden von Fehlern, insbesondere von Fehlern, die sich versehentlich auf andere Teile der Software auswirken, Beibehalten von Regressionstests, frühzeitiges und häufiges Testen usw.).

  2. Installieren Sie den CI-Server mit allen Tests, die bestanden wurden (wenn Sie keine Tests haben, schreiben Sie einen schnellen Test, der bestanden wurde, damit jeder sieht, dass er grün ist).

  3. Richten Sie E-Mails oder andere Benachrichtigungen zum Status der einzelnen Builds ein. Hier ist es wichtig, anzugeben, wer den Code eingecheckt hat, was die Änderung war (oder einen Link zum Commit) und den Status + die Ausgabe des Builds. Dies ist ein wichtiger Schritt, da er eine teamweite Sichtbarkeit sowohl für Erfolge als auch für Misserfolge bewirkt.

  4. Aktualisieren Sie den CI-Server, damit die Tests fehlschlagen, wenn Warnungen angezeigt werden. Dies wird von der Geschäftsleitung und den Teamleitern als systematische Erhöhung der Disziplin des Teams angesehen, und niemand möchte für alle fehlgeschlagenen E-Mails verantwortlich sein.

  5. (optional) Werden Sie nett und frech, indem Sie einige sichtbare Dashboards, Lavalampen, Blinklichter usw. hinzufügen, um den Status des Builds anzuzeigen und zu ermitteln, wer ihn gebrochen / repariert hat.

Ben Taitelbaum
quelle