Sinnloser Code in Ihrer Quelle

Ich habe Geschichten von älteren Programmierern darüber gehört und einige davon selbst gesehen. Es scheint, dass es mehr als ein paar Fälle gibt, in denen Programmierer sinnlosen Code schreiben. Ich werde Dinge sehen wie:

• Methoden- oder Funktionsaufrufe, die keinen Wert haben.
• Redundante Prüfungen in einer separaten Klassendatei, einem Objekt oder einer Methode.
• if Aussagen, die immer wahr sind.
• Fäden, die sich abspinnen und nichts Besonderes tun.

Nur um ein paar zu nennen. Mir wurde gesagt, dass dies darauf zurückzuführen ist, dass Programmierer den Code absichtlich verwirren möchten, um ihren eigenen Wert für das Unternehmen zu steigern, oder sicherstellen möchten, dass bei vertraglichen oder ausgelagerten Arbeiten das Geschäft wiederholt wird.

Meine Frage ist. Hat jemand anderen Code wie diesen gesehen? Was war Ihre Schlussfolgerung, warum dieser Code dort war?

Wenn jemand Code wie diesen geschrieben hat, können Sie uns mitteilen, warum?

Ich habe Entwickler gehört, die versuchen, ihre Codierungserfolge komplexer klingen zu lassen, als sie wirklich sind. Ich habe noch nie gehört, dass jemand dies zugibt, aber ich habe Code gesehen, der Ihre Kriterien erfüllt und der absichtlich aus Eile oder schlechten Praktiken und nicht aus Sabotage erstellt wurde. Der Code, der den fehlerhaften Code umgibt, wurde möglicherweise so geändert, dass eine bestimmte Funktion nicht mehr nützlich ist.

Jemand müsste diesen Code tatsächlich aus erster Hand sehen, bevor er zu dem Schluss kommt, dass nur dieser Entwickler die Komplexität bewältigen kann. Die meisten Manager und andere Geschäftsleute kommen zu diesem Schluss, weil sie keinen Code verstehen und die Position nicht neu besetzen möchten.

Ich habe den Code nicht so gesehen, aber ich habe den Code gesehen, der sinnlos aussieht oder aus anderen Gründen sinnlos ist:

1. Rückwärtskompatibilität. Sie haben einen viel besseren Weg gefunden, Dinge zu tun, aber Sie müssen die alte (und derzeit nicht sehr nützliche) API / Funktion beibehalten, da einige Module von Drittanbietern diese API / Funktion möglicherweise für etwas verwenden. Auch wenn die Funktion nichts Nützliches leistet, kann das Fehlen von Code zu Fehlern führen.

2. Defensive Codierung. Sie wissen, dass die Überprüfungen in diesem Code sinnlos sind, da dies bereits an anderer Stelle überprüft wurde. Aber was ist, wenn jemand diesen Code anderswo ändert und die Prüfungen entfernt oder ändert, damit sie nicht mehr Ihren Voraussetzungen entsprechen?

3. Organisches Wachstum. In großen Projekten ändern sich im Laufe der Jahre viele Dinge, und es stellt sich heraus, dass einige Methoden, die zuvor verwendet wurden, nicht mehr verwendet wurden, aber niemand hat sich die Mühe gemacht, sie zu entfernen, da niemand nachverfolgt hat, ob diese bestimmte Methode verwendet wird oder nicht. Sie wurden lediglich überarbeitet Ihre Code-Teile und zufällig haben sie alle aufgehört, diese Methode zu verwenden. Oder Bedingungen, die einst Bedeutung hatten, aber Anwendung hatten, wurden an anderen Stellen überarbeitet, sodass diese Bedingung immer wahr wurde, aber niemand die Mühe machte, sie zu entfernen.

4. Übergestaltung. Die Leute könnten einige Dinge "nur für den Fall, dass wir es brauchen" und nie wirklich brauchen. Zum Beispiel "Lass uns einen Thread erstellen, falls wir offline arbeiten müssten", und dann fragt niemand, ob er etwas offline machen möchte, und der Programmierer vergisst es und geht zu anderen Projekten (oder vielleicht sogar zu einer anderen Firma) über, und dieser Code bleibt dort für immer, weil niemand weiß, warum es da ist oder ob es sicher ist, es zu entfernen.

Obwohl ich noch nie gesehen habe, dass dies aus Böswilligkeit oder aus falschen Gründen im Hinblick auf die Arbeitsplatzsicherheit geschehen ist, habe ich es unzählige Male erlebt, als es das natürliche Ergebnis der Softwareentwicklung war.

Meine Frage ist. Hat jemand anderen Code wie diesen gesehen? Was war Ihre Schlussfolgerung, warum dieser Code dort war?

1) ja

2) In den Fällen, die ich gesehen habe, würde ich es auf verschiedene Arten aufschreiben:

• Programmierer Unerfahrenheit
• Programmierer, der ein besonders kompliziertes und / oder schlecht ausgeführtes Design nicht versteht, versucht, es zu modifizieren
• Programmierer wird in der Mitte eines Refaktors unterbrochen.
• Nachlässigkeit

Jetzt bin ich vielleicht gemeinnützig, aber mein allgemeiner Ansatz ist, dass es besser ist, in diesen Dingen zu vergeben / nicht konfrontativ zu sein, als mit den Fingern zu zeigen und mit schlechter Qualität weiterzumachen. Offensichtlich könnte es schon schlimm genug werden, dass etwas getan werden muss , aber ein sanfter Schubs in die richtige Richtung ist normalerweise ausreichend.

Natürlich kann man einen solchen Laissez-Faire-Ansatz nicht verfolgen, wenn Qualität / Fehler das "Geschäft" ernsthaft beeinträchtigen. In dieser Situation ist jedoch eine obligatorische und sorgfältige Codeüberprüfung in Verbindung mit einem umfassenden Testverfahren erforderlich.

Meiner Erfahrung nach neigen die Leute dazu, (zumindest teilweise) über minderwertigen Code "nervös" zu werden, weil dies gegen ihre persönlichen Standards verstößt. Es ist sehr gut, (persönlich) nach Perfektion zu streben, aber es ist ein bisschen unvernünftig, Ihre persönlichen Standards auf andere Menschen zu projizieren. Anhand der Geräusche der Dinge (z. B. anhand der Art Ihrer Beispiele) tun Sie dies.

IMO, das ist nicht produktiv und nicht förderlich für eine gute Zusammenarbeit mit Ihren Mitarbeitern.

All dies sind oft Symptome dafür, wie ein Projekt altert.

 1. Methoden- oder Funktionsaufrufe, die keinen Wert haben. Es gibt viele Male, in denen ein Code einfach so belassen wird, wie er ist (hoffentlich mit einer großen veralteten Warnung, aber da die meisten Sprachen diese Warnung nicht haben, wird sie nicht immer befolgt ...), weil er zu einem bestimmten Zeitpunkt einige geliefert hat echte Absicht und niemand wusste, was passieren könnte, wenn die beleidigenden Linien entfernt wurden.

Ich erinnere mich anscheinend an Folgendes aus einer Tageszeitung:

@deprecated // he might have been crazy enough to use reflection...
boolean getTrue() {
    return false; 
}

 2. Redundante Prüfungen, die in einer separaten Klassendatei, einem Objekt oder einer Methode durchgeführt werden. Kommunikationsebenen sind auch unvollkommen (jemals Mythical Man Month gelesen? Wenn nicht, was machst du am Computer? Los! LESEN!). Oft arbeitet eine Person an etwas und verlässt dann das Projekt, und der nächste Typ, der einen bizarren Fehler findet, wirft hier und da einen zusätzlichen Scheck aus, um zu versuchen, ihn zu beseitigen. Wenn der Fehler behoben ist, werden die Überprüfungen nicht durchgeführt, da Sie ihn nicht beheben können, wenn er nicht fehlerfrei ist.

 3. if-Anweisungen, die immer true ergeben. Oh, ich habe das gemacht. Ich habe mal ein Projekt bekommen, es hatte eine Reihe von wahrscheinlich 10-15 if / else Blöcken. Um das Verhalten zu ändern, setze ich einfach einen true||an den ersten Block. Erst Monate (Jahre?) Später kam ich zurück und sagte: "Oh, wow, dieser Code sollte zerstört worden sein, war es aber nie."

 4. Fäden, die sich abspinnen und nichts Besonderes tun. Ich kann mir einen Gedankengang so vorstellen:

1. Ich kenne! Ich kann mit diesen beiden Problemen asynchron umgehen! Ich werde Threads foo und bar erstellen.
2. (zwei Monate später) Weißt du, die Funktionalität von Bar ist in foo ein bisschen besser. Ich werde ein paar umziehen.
3. (Ein Jahr später) Weißt du, es macht Sinn, dieses andere Zeug von der Theke ins Foo zu bringen.
4. (viele, viele Jahre später) "Hey, dieser barThread sieht nicht so aus, als würde er irgendetwas bewirken. Können wir ihn entfernen?" "Besser nicht, es ist schon viele, viele Jahre da ..."

Ich bin eher ein Optimist. Ich denke, was Sie oft beschrieben haben, tritt auf, wenn Code nachlässig überarbeitet wird.

Alte Kollegen erzählten mir von einer Zeit, in der Berater nach der Anzahl der von ihnen erstellten Codezeilen bezahlt wurden . Und so maximierten sie ihre Gewinne mit erstaunlich langwierigen Konstrukten.

Heutzutage gehe ich immer davon aus, dass der Typ während der Arbeit immer noch die Sprache lernt . Und er hat es eilig.

Die meisten Antworten lassen sich auf diese zwei einfachen Fakten zurückführen:
[1] Code spiegelt die Geschichte des Codes wider und
[2] Code spiegelt die erwartete Zukunft des Codes wider.

Ich habe Funktionen geschrieben, die in Anbetracht der aktuellen Spezifikationen nichts von Wert sind, aber möglicherweise in der Zukunft benötigt werden.

Ich habe if-Anweisungen geschrieben, die derzeit immer mit true bewertet werden. Aber vielleicht in der Vergangenheit könnte es falsch sein.

In Bezug auf redundante Überprüfungen, hey, ich vertraue keinem anderen Code, ich vertraue nicht einmal meinem eigenen Code. Wenn ein Modul davon abhängt, dass N 1, 2 oder 3 ist, sollte man dies besser überprüfen und informativ abstürzen, wenn dies nicht der Fall ist. Es ist unzulässig, dass Modul B explodiert, weil Modul A versagt hat. Es ist durchaus legitim, dass Modul B sich beschwert, dass Modul A einen Fehler gemacht hat. Und denken Sie daran, dass dieser Parameter im nächsten Monat möglicherweise aus dem noch nicht geschriebenen Modul C stammt.

Ich habe das schon ein paar Mal gesehen, nämlich erst gestern, als ich einige Codes meiner Chefs in meine neue App einbinden musste. In seinem Fall liegt es nur an einem allgemeinen Mangel an Fähigkeiten und Verständnis und an der Überzeugung, dass er sich für einen ziemlich erfahrenen Entwickler hält.

'Methoden- oder Funktionsaufrufe, die keinen Wert haben.' und 'if-Anweisungen, die immer als wahr ausgewertet werden' sind ein Hauptproblem in seinem Code.

Ich vermute, dass, obwohl viele Code gesehen haben , der diese Probleme aufweist, nur wenige sich dazu bereit erklären würden, dasselbe zu schreiben . Höchstwahrscheinlich handelt es sich bei dem, was Sie sehen, um akkumulierte Softwarefäule - jemand fügt etwas hinzu, was nicht wirklich funktioniert, und der nächste Betreuer fügt weiter hinten in der Kette Schutzcode hinzu, um sich vor dem Zustand zu schützen, der beim ersten Mal nicht ordnungsgemäß überprüft wurde Ort; dann bekommt jemand einen Problembericht und fügt einer bestimmten Instanz eines Problems noch mehr Panzerungen hinzu; Eine andere Person fügt eine allgemeinere Überprüfung hinzu und vergisst, einige der zuvor hinzugefügten alten Codes zu entfernen, die sich mit spezifischeren Symptomen usw. befassten.

Dann gibt es das Problem der Codebereinigung: Es gibt keinen besonderen Anreiz, scheinbar toten Code zu entfernen, und keinen enormen Anreiz, dies nicht zu tun, denn wenn Sie den Code nicht vollständig verstehen, wird Ihre Einschätzung, dass der Code "tot" ist, dazu führen fehlerhaft sein, und Sie werden das System brechen.

• Methoden- oder Funktionsaufrufe, die keinen Wert haben.

Nicht unbedingt schlecht. Methoden in einer Basisklasse rufen häufig leere Methoden auf, die als Überschreibungspunkte für Unterklassen gedacht sind. Beispiel: UIView von Cocoa Touch verfügt über eine -didAddSubview:Methode, bei der in der Standardversion dokumentiert ist , dass sie nichts tut. Die -addSubview:Methode von UIView muss aufgerufen werden -didAddSubview:, obwohl sie nichts tut, da Unterklassen sie möglicherweise implementieren, um etwas zu tun. Methoden, die nichts tun, und die Gründe dafür sollten natürlich dokumentiert werden.

Wenn eine leere oder nutzlose Funktion / Methode aus historischen Gründen offensichtlich vorhanden ist, sollte sie entfernt werden. Schauen Sie sich frühere Versionen des Codes in Ihrem Quellcode-Repository an, wenn Sie sich nicht sicher sind.

• Redundante Prüfungen in einer separaten Klassendatei, einem Objekt oder einer Methode.

Schwer zu sagen, ob das ohne Kontext in Ordnung ist. Wenn die Überprüfungen eindeutig aus demselben Grund durchgeführt werden, kann dies bedeuten, dass die Zuständigkeiten nicht klar voneinander getrennt sind und eine Umgestaltung erforderlich ist, insbesondere, wenn beide Überprüfungen dazu führen, dass dieselbe Maßnahme ergriffen wird. Wenn die Aktion, die sich aus beiden Überprüfungen ergibt, nicht gleich ist, werden die beiden Überprüfungen wahrscheinlich aus unterschiedlichen Gründen durchgeführt, auch wenn die Bedingung gleich ist, und das ist wahrscheinlich in Ordnung.

• if-Anweisungen, die immer true ergeben.

Es gibt einen großen Unterschied zwischen:

if (1) {
    // ...
}

und:

if (foo() == true) {
    // ...
}

wo foo()passiert immer wieder zurückkehren true.

Der erste Fall passiert oft, wenn Leute debuggen. Es ist einfach , eine zu verwenden , if (0) {...um vorübergehend ein Stück Code zu entfernen , während Sie versuchen , einen Fehler zu isolieren und dann die Änderung 0auf 1diesen Code wiederherzustellen. Das ifsollte natürlich entfernt werden, wenn Sie fertig sind, aber es ist leicht, diesen Schritt zu vergessen oder ein oder zwei zu verpassen, wenn Sie es an mehreren Stellen getan haben. (Es ist eine gute Idee, solche Bedingungen mit einem Kommentar zu kennzeichnen, nach dem Sie später suchen können.) Der einzige Schaden ist die Verwirrung, die dies in Zukunft verursachen könnte. Wenn der Compiler den Wert der Bedingung zur Kompilierungszeit ermitteln kann, wird er vollständig entfernt.

Der zweite Fall kann akzeptabel sein. Wenn die durch dargestellte Bedingung foo()an mehreren Stellen im Code getestet werden muss, ist es oft das Richtige, sie in eine separate Funktion oder Methode zu zerlegen, auch wenn sie gerade foo()immer wahr ist. Wenn es vorstellbar ist, dass foo()es irgendwann zu einer Rückkehr kommt false, können Sie durch Isolieren dieser Bedingung in einer Methode oder Funktion alle Stellen identifizieren, an denen sich der Code auf diese Bedingung stützt. Dies birgt jedoch das Risiko, dass die foo() == falseBedingung nicht getestet wird und später zu Problemen führen kann. Die Lösung besteht darin, sicherzustellen, dass Sie Komponententests hinzufügen, die den falseFall explizit testen .

• Fäden, die sich abspinnen und nichts Besonderes tun.

Dies klingt nach einem historischen Artefakt, das entweder während einer Codeüberprüfung oder durch regelmäßige Profilerstellung der Software identifiziert werden kann. Ich nehme an, es könnte absichtlich erstellt werden, aber es fällt mir schwer, mir vorzustellen, dass jemand das absichtlich tun würde.

Es passiert. Eigentlich ziemlich oft.

Manchmal ähneln diese Code-Sackgassen eher alten Ziegenwegen, die verfallen sind, wenn eine effizientere / modernere / schnellere Autobahn um sie herum installiert wurde.

Bei anderen Gelegenheiten (und ich bin wahrscheinlich schuld daran) handelt es sich um Grundlagen für die Erweiterung der Software, wenn eine eingewiesene, aber nicht bestätigte Reihe von Merkmalen / Funktionen angefordert wird. (Manchmal kann das Einarbeiten von ein wenig Arbeit in den anfänglichen Build, der Griffe und dergleichen für spätere Arbeiten bereitstellt, die Sie "anschrauben" möchten, das Leben erleichtern, wenn es passiert, oder schwieriger / unordentlicher sein, wenn die weitere Arbeit dies nicht tut eventuate.)

Und vieles davon ist direkt auf das alte "Wenn es nicht kaputt ist, passt es nicht" zurückzuführen. Manchmal kann das Herausreißen von Code, den Sie nicht verstehen oder der nicht verwendet wird, die Programmierversion von "The Butterfly Effect" verursachen. Habe das schon ein- oder zweimal gemacht.

Manchmal wird ein globaler Boolescher Wert auf true gesetzt, und später in meinem Code wird ein if (bool) angegeben. Dann setze ich zur Laufzeit möglicherweise einen Haltepunkt bei der if-Anweisung und ändere den Booleschen Wert, um etwas zu testen.

Ich widerspreche if trueAussagen, die unterschiedslos als "sinnloser Code" eingestuft werden sollen.

Es gibt einen legitimen Fall für die Verwendung eines if (1) { ... }Blocks in C-Code, der entweder mit dem C-Standard kompatibel sein soll, der darauf besteht, dass Variablendefinitionen am Anfang einer Funktion stehen, oder nur lokale Variablen, die so lokal wie möglich definiert werden sollen.

switch (i) {
    case 23:
        if (1) {
            /* I can declare a local var here! */
        }
        break;
}

Ein Professor von mir erzählte uns eines Tages die Geschichte, dass ein früherer Arbeitgeber sie basierend auf der Anzahl der Zeilen, die sie ausgefüllt haben, bezahlt hätte. Also schrieben sie mehrere Dutzend Funktionen, die nie aufgerufen wurden. Scheint ein guter Grund zu sein, sinnlosen Code zu schreiben.

Wie @Andy bereits sagte, ist eine große Komponente, die ich gesehen habe, das Brechen von YAGNI .

Jemand beginnt mit einer Annahme, was alle Elemente sein werden anstatt wie viele Elemente benötigt werden. Dies ist eine Verwechslung der Beziehungen "ist ein" und "hat ein" .

Diese Verwirrung führt zu einer starren Struktur der Vererbung, und dann handelt es sich um Methoden, die nicht implementiert werden, weil sie nie aufgerufen werden, um wiederholte Logik, bei der Teile optimiert werden müssen, und um im Allgemeinen seltsame Workflows, die sich nicht am Geschäftsmodell ausrichten.

Wie viele andere hier habe ich das nicht böswillig gesehen, aber aus Mangel an Wissen über gutes Design und dem Versuch, es für andere so aussehen zu lassen. Komisch, es scheint, dass Entwickler, die noch weniger über Kenntnisse verfügen, diesbezüglich besser abschneiden, da zumindest ihr Code nicht übermäßig entwickelt wird. ( KISS-Prinzip )