it-swarm-eu.dev

Welche Dinge läuten sofort Alarmglocken, wenn Sie sich den Code ansehen?

Ich habe vor ein paar Wochen an einer Software-Handwerksveranstaltung teilgenommen und einer der Kommentare lautete: "Ich bin sicher, wir alle erkennen schlechten Code, wenn wir ihn sehen", und alle nickten ohne weitere Diskussion klug.

Diese Art von Dingen macht mir immer Sorgen, da es diese Binsenweisheit gibt, die jeder für einen überdurchschnittlichen Fahrer hält. Obwohl ich denke, dass ich schlechten Code erkennen kann, würde ich gerne mehr darüber erfahren, was andere Leute als Code-Gerüche betrachten, da dies in den Blogs der Leute selten und nur in einer Handvoll Büchern ausführlich besprochen wird. Insbesondere denke ich, dass es interessant wäre, etwas zu hören, das in einer Sprache nach Code riecht, in einer anderen jedoch nicht.

Ich werde mit einem einfachen beginnen:

Code in der Quellcodeverwaltung, der einen hohen Anteil an auskommentiertem Code enthält - warum ist er dort? sollte es gelöscht werden? ist es eine halbfertige Arbeit? Vielleicht hätte es nicht auskommentiert werden sollen und wurde nur gemacht, wenn jemand etwas ausprobierte? Persönlich finde ich so etwas wirklich ärgerlich, auch wenn es hier und da nur die eine oder andere Zeile ist, aber wenn man große Blöcke sieht, die mit dem Rest des Codes durchsetzt sind, ist das völlig inakzeptabel. Dies ist normalerweise auch ein Hinweis darauf, dass der Rest des Codes wahrscheinlich ebenfalls von zweifelhafter Qualität ist.

98
FinnNk
/* Fuck this error */

In der Regel in einem Unsinn gefunden try..catch Block, es neigt dazu, meine Aufmerksamkeit zu erregen. Fast so gut wie /* Not sure what this does, but removing it breaks the build */.

Noch ein paar Dinge:

  • Mehrere verschachtelte komplexe if Anweisungen
  • Try-Catch-Blöcke, mit denen regelmäßig ein Logikfluss ermittelt wird
  • Funktionen mit generischen Namen process, data, change, rework, modify
  • Sechs oder sieben verschiedene Aussteifungsstile in 100 Linien

Eine, die ich gerade gefunden habe:

/* Stupid database */
$conn = null;
while(!$conn) {
    $conn = mysql_connect("localhost", "root", "[pass removed]");
}
/* Finally! */
echo("Connected successfully.");

Richtig, denn wenn Sie Ihre MySQL-Verbindungen brutal erzwingen müssen, ist dies der richtige Weg. Es stellte sich heraus, dass die Datenbank Probleme mit der Anzahl der Verbindungen hatte, sodass das Zeitlimit überschritten wurde. Anstatt dies zu debuggen, versuchten sie es einfach immer wieder, bis es funktionierte.

128
Josh K

Die wichtigste rote Fahne für mich sind doppelte Codeblöcke, da sie zeigen, dass die Person entweder die Programmiergrundlagen nicht versteht oder zu ängstlich war, um die richtigen Änderungen an einer vorhandenen Codebasis vorzunehmen.

Früher habe ich auch fehlende Kommentare als rote Fahne gewertet, aber nachdem ich kürzlich an vielen sehr guten Codes ohne Kommentare gearbeitet habe, habe ich darauf zurückgegriffen.

104
Ben Hughes

Code, der zu zeigen versucht, wie klug der Programmierer ist, obwohl er keinen wirklichen Wert hinzufügt:

x ^= y ^= x ^= y;
74
Rei Miyasaka
  • 20.000 (übertriebene) Zeilenfunktionen. Jede Funktion, die mehr als ein paar Bildschirme benötigt, muss neu faktorisiert werden.

  • In diesem Sinne Klassendateien, die scheinbar für immer weitergehen. Es gibt wahrscheinlich einige Konzepte, die in Klassen abstrahiert werden könnten, um den Zweck und die Funktion der ursprünglichen Klasse zu klären, und wahrscheinlich, wo sie verwendet wird, es sei denn, es handelt sich um interne Methoden.

  • nicht beschreibende, nicht triviale Variablen oder zu viele triviale nicht beschreibende Variablen. Diese machen das Ableiten dessen, was tatsächlich passiert, zu einem Rätsel.

62
{ Let it Leak, people have good enough computers to cope these days }

Schlimmer ist, dass es aus einer kommerziellen Bibliothek stammt!

61
Reallyethical

Kommentare, die so ausführlich sind, dass ein englischer Compiler perfekt kompiliert und ausgeführt wird, aber nichts beschreibt, was der Code nicht beschreibt.

//Copy the value of y to temp.
temp = y;
//Copy the value of x to y.
y = x;
//Copy the value of temp to x.
x = temp;

Auch Kommentare zu Code, die hätten entfernt werden können, wenn der Code einige grundlegende Richtlinien eingehalten hätte:

//Set the age of the user to 13.
a = 13;
53
Rei Miyasaka

Code, der beim Kompilieren Warnungen erzeugt.

42
Rei Miyasaka

Funktionen mit Zahlen im Namen anstelle von beschreibenden Namen, wie:

void doSomething()
{
}

void doSomething2()
{
}

Bitte lassen Sie die Funktionsnamen etwas bedeuten! Wenn doSomething und doSomething2 ähnliche Aktionen ausführen, verwenden Sie Funktionsnamen, die die Unterschiede unterscheiden. Wenn doSomething2 eine Unterbrechung der Funktionalität von doSomething darstellt, benennen Sie es für seine Funktionalität.

36
Wonko the Sane

Magic Numbers oder Magic Strings.

   if (accountBalance>200) { sendInvoice(...); }

   salesPrice *= 0.9;   //apply discount    

   if (invoiceStatus=="Overdue") { reportToCreditAgency(); }
36
JohnFx
  • Vielleicht nicht das Schlimmste, zeigt aber deutlich die Ebene der Implementierer:

    if(something == true) 
    
  • Wenn eine Sprache über eine for-Schleife oder ein Iterator-Konstrukt verfügt, zeigt die Verwendung einer while-Schleife auch das Verständnis der Implementierer für die Sprache:

    count = 0; 
    while(count < items.size()){
       do stuff
       count ++; 
    }
    
    for(i = 0; i < items.size(); i++){
      do stuff 
    }
    //Sure this is not terrible but use the language the way it was meant to be used.
    
  • Schlechte Rechtschreibung/Grammatik in Dokumentation/Kommentaren frisst mich fast so viel wie Code selbst. Der Grund dafür ist, dass Code für Menschen zum Lesen und zum Ausführen von Maschinen gedacht war. Aus diesem Grund verwenden wir Hochsprachen. Wenn Ihre Dokumentation schwer zu verarbeiten ist, kann ich mir präventiv eine negative Meinung über die Codebasis bilden, ohne sie zu betrachten.

36
Chris

Diejenige, die ich sofort bemerke, ist die Häufigkeit von tief verschachtelte Codeblöcke (if's, while's usw.). Wenn Code häufig mehr als zwei oder drei Ebenen tief ist, ist dies ein Zeichen für ein Design-/Logikproblem. Und wenn es wie 8 Nester tief geht, gibt es besser einen verdammt guten Grund dafür, dass es nicht zerbrochen wird.

29
GrandmasterB

Wenn ich das Programm eines Schülers benote, kann ich es manchmal im "Blink" -Stil erkennen. Dies sind die augenblicklichen Hinweise:

  • Schlechte oder inkonsistente Formatierung
  • Mehr als zwei Leerzeilen hintereinander
  • Nicht standardmäßige oder inkonsistente Namenskonventionen
  • Wiederholter Code, je wörtlicher die Wiederholungen, desto stärker die Warnung
  • Was ein einfacher Code sein sollte, ist zu kompliziert (z. B. die verschlungene Überprüfung der an main übergebenen Argumente)

Selten sind meine ersten Eindrücke falsch, und diese Warnglocken stimmen ungefähr 95% der Zeit. Für eine Ausnahme verwendete ein Schüler, der neu in der Sprache war, einen Stil aus einer anderen Programmiersprache. Durch das Eingraben und erneutes Lesen ihres Stils in der Sprache der anderen Sprache wurden die Alarmglocken für mich entfernt, und der Schüler erhielt dann die volle Anerkennung. Solche Ausnahmen sind jedoch selten.

Wenn ich fortgeschritteneren Code betrachte, sind dies meine anderen Warnungen:

  • Das Vorhandensein von vielen Java Klassen, die nur "Strukturen" sind, um Daten zu speichern. Es spielt keine Rolle, ob die Felder sind öffentlich oder privat und verwenden Getter/Setter. Dies ist wahrscheinlich immer noch nicht Teil eines gut durchdachten Designs.
  • Klassen mit schlechten Namen, z. B. nur ein Namespace, und der Code enthält keinen wirklichen Zusammenhalt
  • Verweis auf Entwurfsmuster, die nicht einmal im Code verwendet werden
  • Leere Ausnahmebehandlungsroutinen ohne Erklärung
  • Wenn ich den Code in Eclipse aufrufe, säumen Hunderte von gelben "Warnungen" den Code, hauptsächlich aufgrund nicht verwendeter Importe oder Variablen

In Bezug auf den Stil mag ich es im Allgemeinen nicht zu sehen:

  • Javadoc-Kommentare, die nur den Code wiedergeben

Dies sind nur Hinweise auf schlechten Code. Manchmal ist das, was als schlechter Code erscheint, wirklich nicht so, weil Sie die Absichten des Programmierers nicht kennen. Zum Beispiel kann es einen guten Grund geben, warum etwas zu komplex erscheint - es könnte eine andere Überlegung im Spiel gewesen sein.

28
Macneil

Persönlicher Favorit/Pet Peeve: IDE generierte Namen, die festgelegt werden. Wenn TextBox1 eine wichtige und wichtige Variable in Ihrem System ist, steht eine weitere Überprüfung des Codes an.

25
Wyatt Barnett

Völlig unbenutzte Variablen, insbesondere wenn die Variable einen Namen hat, der den verwendeten Variablennamen ähnelt.

25
C. Ross

Viele Leute haben erwähnt:

//TODO: [something not implemented]

Ich wünschte, das Zeug wäre implementiert worden, aber zumindest haben sie sich eine Notiz gemacht. Was ich für schlimmer halte, ist:

//TODO: [something that is already implemented]

Todos sind wertlos und verwirrend, wenn Sie sich nie die Mühe machen, sie zu entfernen!

21

Konjunktionen in Methodennamen:

public void addEmployeeAndUpdatePayrate(...) 


public int getSalaryOrHourlyPay(int Employee) ....

Klarstellung: Der Grund, warum dies Alarmglocken läutet, ist, dass es anzeigt, dass die Methode wahrscheinlich gegen das Prinzip der Einzelverantwortung verstößt.

20
JohnFx

Eine Methode, bei der ich nach unten scrollen muss, um alles zu lesen.

20
BradB

Offensichtlich verknüpften GPL-Quellcode mit einem kommerziellen Closed-Source-Programm.

Dies stellt nicht nur ein unmittelbares rechtliches Problem dar, sondern weist meiner Erfahrung nach normalerweise entweder auf Nachlässigkeit oder Unbesorgtheit hin, was sich auch an anderer Stelle im Code widerspiegelt.

13
Bob Murphy

Sprachunabhängig :

  • TODO: not implemented
  • int function(...) { return -1; } (das gleiche wie "nicht implementiert")
  • Ausnahmeregelung aus einem nicht außergewöhnlichen Grund.
  • Missbrauch oder inkonsistente Verwendung von 0, -1 Oder null als außergewöhnliche Rückgabewerte.
  • Behauptungen ohne überzeugenden Kommentar, warum es niemals scheitern sollte.

Sprachspezifisch (C++) :

  • C++ - Makros in Kleinbuchstaben.
  • Statische oder globale C++ - Variablen.
  • Nicht initialisierte oder nicht verwendete Variablen.
  • Alle array new, Die anscheinend nicht RAII-sicher sind.
  • Jede Verwendung von Arrays oder Zeigern, die anscheinend nicht grenzenlos sind. Dies beinhaltet printf.
  • Jede Verwendung des nicht initialisierten Teils eines Arrays.

Microsoft C++ spezifisch :

  • Alle Bezeichnernamen, die mit einem Makro kollidieren, das bereits in einer der Microsoft SDK-Headerdateien definiert ist.
  • Im Allgemeinen ist jede Verwendung der Win32-API eine große Quelle für Alarmglocken. Lassen Sie MSDN immer geöffnet und suchen Sie im Zweifelsfall nach den Argumenten/Rückgabewertdefinitionen. (bearbeitet)

C++/OOP-spezifisch :

  • Implementierungsvererbung (konkrete Klasse), bei der die übergeordnete Klasse sowohl über virtuelle als auch über nicht virtuelle Methoden verfügt, ohne dass eine klare offensichtliche konzeptionelle Unterscheidung zwischen dem, was virtuell sein sollte oder nicht, erfolgt.
9
rwong

Verwenden Sie viele Textblöcke anstelle von Aufzählungen oder global definierten Variablen.

Nicht gut:

if (itemType == "Student") { ... }

Besser:

private enum itemTypeEnum {
  Student,
  Teacher
}
if (itemType == itemTypeEnum.Student.ToString()) { ... }

Beste:

private itemTypeEnum itemType;
...
if (itemType == itemTypeEnum.Student) { ... }
8
Yaakov Ellis

Bizarrer Einrückungsstil.

Es gibt ein paar sehr beliebte Stile, und die Leute werden diese Debatte ins Grab bringen. Aber manchmal sehe ich jemanden, der einen wirklich seltenen oder sogar selbstgebauten Einrückungsstil verwendet. Dies ist ein Zeichen dafür, dass sie wahrscheinlich nur mit sich selbst codiert haben.

8
Ken

Schwach typisierte Parameter oder Rückgabewerte für Methoden.

public DataTable GetEmployees() {...}

public DateTime getHireDate(DataTable EmployeeInfo) {...}

public void FireEmployee(Object EmployeeObjectOrEmployeeID) {...}
8
JohnFx

Code-Geruch: Best Practices nicht befolgen

Diese Art von Dingen macht mir immer Sorgen, da es diese Binsenweisheit gibt, die jeder für einen überdurchschnittlichen Fahrer hält.

Hier ist ein Nachrichtenblitz für Sie: 50% der Weltbevölkerung sind unterdurchschnittlich intelligent. Ok, einige Leute hätten genau die durchschnittliche Intelligenz, aber lasst uns nicht wählerisch werden. Einer der Nebeneffekte der Dummheit ist auch, dass Sie Ihre eigene Dummheit nicht erkennen können! Die Dinge sehen nicht so gut aus, wenn Sie diese Aussagen kombinieren.

Welche Dinge läuten beim Betrachten des Codes sofort Alarmglocken?

Viele gute Dinge wurden erwähnt, und im Allgemeinen scheint es, dass nicht den Best Practices folgen Ein Codegeruch ist.

Best Practices werden normalerweise nicht zufällig erfunden und sind oft aus einem bestimmten Grund vorhanden. Oft kann es subjektiv sein, aber meiner Erfahrung nach sind sie meistens gerechtfertigt. Das Befolgen von Best Practices sollte kein Problem sein, und wenn Sie sich fragen, warum sie so sind, wie sie sind, recherchieren Sie sie, anstatt sie zu ignorieren und/oder sich darüber zu beschweren - vielleicht ist es gerechtfertigt, vielleicht nicht.

Ein Beispiel für eine bewährte Methode könnte die Verwendung von Locken mit jedem if-Block sein, selbst wenn er nur eine Zeile enthält:

if (something) {
    // whatever
}

Sie denken vielleicht nicht, dass es notwendig ist, aber ich habe kürzlich gelesen , dass es eine Hauptquelle für Fehler ist. Die Verwendung von Klammern wurde auch unter Stapelüberlauf erläutert, und die Überprüfung, ob if-Anweisungen Klammern haben, ist auch eine Regel in PMD , ein statischer Code-Analysator für Java.

Denken Sie daran: "Weil es sich um eine bewährte Methode handelt" ist niemals eine akzeptable Antwort auf die Frage "Warum machen Sie das?" Wenn Sie nicht artikulieren können, warum etwas eine bewährte Methode ist, dann ist es keine bewährte Methode, sondern ein Aberglaube.

7
Vetle
  • Mehrere ternäre Operatoren reihen sich aneinander, sodass sie nicht einem if...else - Block ähneln, sondern zu einem if...else if...[...]...else - Block werden
  • Lange Variablennamen ohne Unterstriche oder camelCasing. Beispiel aus einem Code, den ich aufgerufen habe: $lesseeloginaccountservice
  • Hunderte von Codezeilen in einer Datei, mit wenig bis gar keinen Kommentaren, und der Code ist sehr offensichtlich
  • Übermäßig komplizierte if Anweisungen. Beispiel aus Code: if (!($lessee_obj instanceof Lessee && $lessee_obj != NULL)), das ich auf if ($lessee_obj == null) heruntergekaut habe
7
Tarka

Allgemeine Ausnahmen abfangen :

try {

 ...

} catch {
}

oder

try {

 ...

} catch (Exception ex) {
}

Überbeanspruchung der Region

Wenn Sie zu viele Regionen verwenden, bedeutet dies normalerweise, dass Ihre Klassen zu groß sind. Es ist ein Warnflag, das signalisiert, dass ich mich eingehender mit diesem Code befassen sollte.

6
Erik van Brakel

Kann sich jemand ein Beispiel vorstellen, bei dem Code legitimerweise über einen absoluten Pfad auf eine Datei verweisen sollte?

6
Rei Miyasaka

Kommentare, die besagen, "das liegt daran, dass das Design des froz-Subsystems völlig durcheinander ist."

Das geht über einen ganzen Absatz.

Sie erklären, dass der folgende Refactor passieren muss.

Aber habe es nicht getan.

Jetzt wurde ihnen vielleicht gesagt, dass sie es zu diesem Zeitpunkt von ihrem Chef aufgrund von Zeit- oder Kompetenzproblemen nicht ändern konnten, aber vielleicht lag es daran, dass die Leute kleinlich waren.

Wenn ein Vorgesetzter denkt, dass j.random. Der Programmierer kann kein Refactoring durchführen, dann sollte der Supervisor dies tun.

Wie auch immer, ich weiß, dass der Code von einem geteilten Team mit möglicher Machtpolitik geschrieben wurde, und sie haben borked Subsystem-Designs nicht überarbeitet.

Wahre Geschichte. Das könnte dir geschehen.

6
Tim Williscroft
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...

Natürlich ohne jegliche Dokumentation und gelegentlich verschachtelte #defines

5
Sven

C++ - Code mit expliziten Löschanweisungen (es sei denn, ich betrachte die Innereien einer Smart-Pointer-Implementierung). 'delete' ist das 'goto' der Speicherverwaltung IMHO .

Eng damit verbunden, Code wie:

// Caller must take ownership
Thing* Foo::Bar()

(Und Sie können sich glücklich schätzen, wenn es den Kommentar gibt!). Es ist nicht so, dass intelligente Zeiger Raketenwissenschaft sind. std::auto_ptr wurde für diese Art von Dingen erstellt (Dokumentieren und Erzwingen der im Kommentar ausgedrückten Absicht) und war Teil des Standard = seit Ewigkeiten.

Zusammen schreien diese ungeliebten Legacy-Codes oder Betreuer mit einer Denkweise, die irgendwo in den frühen 90ern stecken geblieben ist.

5
timday

Funktionen, die die Grundfunktionalität der Sprache neu implementieren. Wenn Sie beispielsweise jemals eine "getStringLength ()" - Methode in JavaScript anstelle eines Aufrufs der ".length" -Eigenschaft eines Strings sehen, wissen Sie, dass Sie in Schwierigkeiten sind.

5
Ant

Klassennamenkonventionen, die ein schlechtes Verständnis der Abstraktion zeigen, die sie erstellen möchten. Oder das definiert überhaupt keine Abstraktion.

Ein extremes Beispiel fällt mir in einer VB Klasse, die ich einmal gesehen habe, die den Titel Data trug und mehr als 30.000 Zeilen lang war ... in der ersten Datei. Es war eine Teilklasse, die in mindestens ein halbes Dutzend anderer Dateien aufgeteilt war. Die meisten Methoden waren Wrapper um gespeicherte Prozesse mit Namen wie FindXByYWithZ().

Selbst mit weniger dramatischen Beispielen bin ich sicher, dass wir alle die Logik nur in eine schlecht konzipierte Klasse "geworfen" haben, ihr einen ganz generischen Titel gegeben und sie später bereut haben.

5
Bryan M.
ON ERROR RESUME NEXT

Die Wartung von Classic ASP -Anwendungen) ist für die meisten ASP.NET-Entwickler leider eine Notwendigkeit, aber das Öffnen einer gemeinsamen Include-Datei und das Erkennen, dass dies in der ersten Zeile die Seele zerstört.

4
richeym

Wenn es keinerlei Kommentare oder Dokumentationen darüber gibt, was der Code tut oder tun soll (d. H. "Der Code ist die Dokumentation").

Methoden oder Variablen mit einer Zahl als Suffix (z. B. Login2 ()).

4
leson
try
{
//do something
}
catch{}
3
Tom Squires

Code, der niemals logisch den Ausführungspfad eingeben kann.

var product = repository.FindProduct(id);

log.Info("Found product " + product.Name);

if (product != null)
{
    // This will always happen
}
else
{
    // **This won't ever happen**
}

oder

if (messages.Count > 0)
{
    // Do stuff with messages
}
else if (messages.Count == 1)
{
    // **I hope this code isn't important...**
}
3
rmac
  • Setzen Sie jede lokale Variable in die ersten paar Zeilen des Methodenblocks. Besonders in Verbindung mit langen Methoden.
  • Verwenden von booleschen Variablen zum Ausbrechen von Schleifen/Überspringen von Iterationen, anstatt nur break/continue zu verwenden
3
Oliver Weiler

Aus meiner Java-zentrierten Perspektive:

  • Nicht standardmäßiger Codierungsstil.
  • Nicht private Variablen.
  • Fehlendes final in Feldern.
  • Sinnlos oder Überbeanspruchung der Vererbung.
  • Riesige Klassen oder Codeblöcke.
  • Zu viele Kommentare (sie sind wahrscheinlich sowieso nur Wunschdenken).
  • Unstrukturierte Protokollierung.
  • Getter und Setter (Schnittstellen sollten sich mit Verhalten befassen).
  • Doppelte Daten.
  • Seltsame Abhängigkeiten.
  • Statik, einschließlich Thread-Globals.
  • Bei Multithread-Code Teile derselben Klasse, von denen erwartet wird, dass sie in verschiedenen Threads ausgeführt werden (insbesondere GUI-Code).
  • Toter Code.
  • String-Manipulation gemischt mit anderem Code.
  • Im Allgemeinen werden Schichten gemischt (übergeordnetes Material, kombiniert mit Iteration über eine Reihe von Grundelementen oder beispielsweise Fadenbehandlung).
  • Jede Verwendung von Reflexion.
  • catch Blöcke ohne nützlichen Code (schlecht: Kommentare, printf, Protokollierung oder einfach leer).

Verwenden eines versteckten Objekts in der Benutzeroberfläche (z. B. eines Textfelds) zum Speichern eines Werts, anstatt eine Variable mit ordnungsgemäßem Gültigkeitsbereich und Typ zu definieren.

2
MartW

Immer wenn ich folgendes lese:

//Don't put in negative numbers or it will crash the program.

Oder etwas ähnliches. Wenn dies der Fall ist, geben Sie eine Behauptung ab! Lassen Sie den Debugger wissen, dass Sie diese Werte zur Laufzeit nicht möchten, und stellen Sie sicher, dass der Code den Vertrag mit dem Aufrufer enthält.

2
wheaties

Diese Art von Code:

        if (pflayerdef.DefinitionExpression == "NM_FIELD = '  '" || One.Two.nmEA == "" || 
            One.Two.nmEA == " " || One.Two.nmEA == null ||
            One.Two.nmEA == "  ")
        {                
            MessageBox.Show("BAD CODE");
            return;
        }

Dies ist aus einer echten Live-Produktions-Codebasis!

2
George Silva

Was magische Zahlen betrifft: Sie sind schlecht, wenn sie an verschiedenen Orten verwendet werden. Wenn Sie sie ändern, müssen Sie sie an mehreren Stellen synchronisieren. Aber eine Zahl an einem Ort ist nicht schlechter als eine Konstante, um eine Zahl zu bezeichnen, die noch an einem Ort verwendet wird.

Darüber hinaus haben Konstanten möglicherweise nicht viel Platz in Ihrer Anwendung. In vielen Datenbank-Apps sollten diese Dinge gemäß App oder Benutzereinstellungen in der Datenbank gespeichert werden. Und um ihre Implementierung abzuschließen, beinhalten sie diese Einstellung und einen Platz in der Benutzeroberfläche sowie einen Hinweis in der Endbenutzerdokumentation. All dies ist eine Art Überentwicklung und eine Verschwendung von Ressourcen, wenn alle vollkommen zufrieden sind, wenn die Zahl 5 ist ( und 5 ist es.)

Ich denke, Sie können Zahlen und Zeichenfolgen an Ort und Stelle lassen, bis diese Nummer außerhalb dieser Stelle verwendet werden muss. Dann ist es Zeit, die Dinge zu einem flexibleren Design umzugestalten.

Was Strings betrifft ... Ich kenne die Argumente, aber dies ist ein weiterer Ort, an dem es keinen Sinn macht, eine Konvertierung von einem String in eine Konstante durchzuführen. Insbesondere, wenn die vorhandenen Zeichenfolgen ohnehin aus einer konstanten Implementierung stammen (z. B. Importe, die von einer externen Anwendung generiert werden und eine Statuszeichenfolge haben, die kurz und erkennbar ist, genau wie "Überfällig") Viel Sinn beim Konvertieren von 'Overdue' in STATUS_OVERDUE, wenn es ohnehin nur an einer Stelle verwendet wird.

Ich bin sehr dafür, keine Komplexität hinzuzufügen, wenn dies nicht die erforderlichen Vorteile für Flexibilität, Wiederverwendung oder Fehlerprüfung schafft. Wenn Sie die Flexibilität benötigen, codieren Sie sie richtig (das Refactor-Ding). Aber wenn es nicht gebraucht wird ...

2
Inca

Eng gekoppelter Code. Besonders wenn Sie viele fest codierte Dinge (Namen von Netzwerkdruckern, IP-Adressen usw.) in der Mitte des Codes sehen. Diese sollten sich in einer Konfigurationsdatei oder sogar in Konstanten befinden, aber das Folgende wird letztendlich nur Probleme verursachen:

if (Host_ip == '192.168.1.5'){
   printer = '192.168.1.123';
} else
  printer = 'prntrBob';

Eines Tages wird Bob aufhören und sein Drucker wird umbenannt. Eines Tages erhält der Drucker eine neue IP-Adresse. Eines Tages wird 192.168.1.5 auf Bobs Drucker drucken wollen.

mein Lieblingsmantra: Schreibe immer Code wie ein Mordpsychopath, der weiß, wo du lebst, muss ihn pflegen!

2
davidhaskins

Code, der zeigt, dass sich der Programmierer vor Jahren nie an Java 5:

  • Verwendung von Iteratoren anstelle von "für jeden"
  • Keine Verwendung von Generika in Sammlungen und Umwandeln der abgerufenen Objekte in den erwarteten Typ
  • Verwenden Sie alte Klassen wie Vector und Hashtable

Ich kenne die modernen Multithread-Methoden nicht.

2
Dave Briccetti

Für SQL:

Der erste große Indikator ist die Verwendung impliziter Verknüpfungen.

Als nächstes wird ein linker Join in Tabelle B verwendet, kombiniert mit einer WHERE-Klausel wie:

WHERE TableB.myField = 'test'

Wenn Sie nicht wissen, dass dies zu falschen Ergebnissen führt, kann ich nicht darauf vertrauen, dass jede von Ihnen geschriebene Abfrage zu korrekten Ergebnissen führt.

2
HLGEM

Mit unserem alten VB6-Code können Sie jedes Modul oder jede Formular-Codepage öffnen und einen Bildschirm voller öffentlicher oder globaler # & @ ! -Variablen finden, die oben deklariert sind und von überall auf @ &! Verweisen! (*! # Programm. ARGH !!!!

(Puh, ich musste das rausholen :-))

2
HardCode

Etwas wie das

x.y.z.a[b].c

Das riecht nach Bio-Gefahr. So viele Mitgliederreferenzen sind niemals ein gutes Zeichen. Und ja, dies ist ein typischer Ausdruck in dem Code, mit dem ich arbeite.

2
Gaurav

alles mit so etwas

// TODO: anything could be here!

Bearbeiten: Ich sollte mich qualifizieren, dass ich dies im Produktionscode gemeint habe. Aber selbst in Code, der der Quellcodeverwaltung verpflichtet ist, ist dies immer noch nicht gut. Aber das könnte eine persönliche Sache sein, da ich den freien Tag gerne beenden möchte, nachdem ich alle meine losen Enden abgebunden habe :)

Edit 2: Ich sollte weiter qualifizieren, dass ich meinte, wenn ich dies in etabliertem Code sehe. Wie etwas, das einige Jahre alt ist und ich behebe einen Fehler. Ich sehe einen TODO und dann läuten die Alarmglocken. TODOs (für mich) implizieren, dass der Code aus irgendeinem Grund nie fertiggestellt wurde.

2
Antony Scott

Die Verwendung des Schlüsselworts synchronized in Java.

Nicht, dass irgendetwas falsch daran ist, synchronized zu verwenden, wenn es richtig verwendet wird. Aber in dem Code, mit dem ich arbeite, scheint es, dass jedes Mal, wenn jemand versucht, threadsicheren Code zu schreiben, er etwas falsch macht. Ich weiß also, dass ich, wenn ich dieses Schlüsselwort sehe, besonders vorsichtig mit dem Rest des Codes sein muss ...

1
Guillaume

Gucklochoptimierungen für Code, die mit einer besseren Struktur optimiert werden könnten, z. In Inline Assembly implementierte lineare Suchen, wenn eine binäre Suche in einfachem C/C++/Java/C # angemessen (und schneller) wäre.

Entweder fehlen der Person einige grundlegende Konzepte oder sie hat keinen Sinn für Prioritäten. Letzteres ist viel besorgniserregender.

1
Rei Miyasaka

@FinnNk, ich stimme Ihnen in Bezug auf auskommentierten Code zu. Was mich wirklich nervt, sind solche Sachen:

for (var i = 0; i < num; i++) {
    //alert(i);
}

oder irgendetwas, das zum Testen oder Debuggen da war und auskommentiert und dann festgeschrieben wurde. Wenn es nur gelegentlich vorkommt, ist es keine große Sache, aber wenn es überall ist, überfrachtet es den Code und macht es schwierig zu sehen, was los ist.

1
Elias Zamaria
  • $ data - Es ist wie Werbung für "Physische Objekte, jetzt bei lächerlich niedrigen 100 pro 5!"
  • TODO-Elemente im Code - Verwenden Sie einen Bug-/Ticket-/Issue-Tracker, damit die Benutzer wissen, was auf Produktebene und nicht auf Dateiebene benötigt wird.
  • Arbeitsanmeldecode - Dafür ist die Versionskontrolle gedacht.
1
l0b0

Alles, was gegen wichtige Prinzipien verstößt. Zum Beispiel wurden einige Anti-Muster vorgeschlagen (magische Zahl - siehe http://en.wikipedia.org/wiki/Anti-pattern ). Anti-Muster werden so genannt, weil sie Probleme verursachen (auch bereits erwähnt - Zerbrechlichkeit, Albträume bei der Wartung usw.). Achten Sie auch auf Verstöße gegen die Prinzipien von SOLID - siehe http://en.wikipedia.org/wiki/Solid_ (object-oriented_design) Auch Code, der dies nicht tut Berücksichtigen Sie nicht die Trennung von Ebenen (Datenzugriff innerhalb der Benutzeroberfläche usw.). Codierungsstandards und Codeüberprüfungen helfen dabei, dies zu bekämpfen.

1
Tim Claason

Die meisten davon stammen aus Java Erfahrung:

  • String-Eingabe. Einfach nein.
  • Typecasting weist in modernem Java häufig auf einen Codegeruch hin.
  • Das Anti-Pattern der Pokemon-Ausnahme (wenn du sie alle fangen musst).
  • Frachtkult versucht funktionale Programmierung, wo es nicht angebracht ist.
  • Verwenden Sie kein funktionales Konstrukt (Callable, Function usw.), wo dies angemessen wäre.
  • Polymorphismus nicht ausnutzen.
1
Ben Hardy

Wenn der Code wie ein Durcheinander aussieht: Kommentare mit "todo" und "note to self" und lahme Witze. Code, der offensichtlich irgendwann ausschließlich zu Debugging-Zwecken verwendet wurde, dann aber nicht entfernt wurde. Variablen oder Funktionen mit Namen, die darauf hindeuten, dass der Verfasser nicht daran gedacht hat, dass jemand sie später lesen würde. Oft sind diese Namen sehr lang und unhandlich.

Außerdem: Wenn dem Code der Rhythmus fehlt. Funktionen von sehr unterschiedlicher Länge. Funktionen, die nicht denselben Namensschemata entsprechen.

Leicht verwandt: Es macht mich immer nervös, wenn ein Programmierer eine schlampige Handschrift hat. Oder wenn sie schlechte Schriftsteller oder schlechte Kommunikatoren sind.

1
KaptajnKold

Ich habe einmal an einem Projekt gearbeitet, bei dem der Auftragnehmer jeden Standarddatentyp von int bis string einschließlich Zeigern auf undurchsichtige Namen definiert hatte. Sein Ansatz machte es wirklich schwierig, den Code zu verstehen. Ein anderer Stil, der mich warnt, ist vorzeitige Flexibilität; Ein Produkt, an dem ich einmal gearbeitet habe, hatte den vollständigen Code in DLLs, die in keiner vorhersehbaren Reihenfolge geladen wurden. All dies, um der zukünftigen Entwicklung Rechnung zu tragen. Einige DLLs verwendeten Thread-Wrapper für die Portabilität. Es war ein Albtraum, das Programm zu debuggen oder den Ablauf durch Lesen des Quellcodes vorherzusagen. Die Definitionen waren über die Header-Dateien verteilt. Es war keine Überraschung, dass der Code nicht über die zweite Version hinaus überlebte.

1
VKs

Manchmal sehe ich Teile einer Methode, die bei allen möglichen Eingaben NIEMALS ausgeführt wird, sodass sie die Leute überhaupt nicht verwirren sollte. Ein Beispiel wäre ...
Wenn die Methode nur im Kontext eines Admin-Superusers aufgerufen werden kann und ich sehe, dass überprüft wird, ob der Anforderungsbenutzer kein Admin-Superuser ist ...: /

1
chiurox

Verärgerte Kommentare zeigen mangelnde Zurückhaltung:

//I CAN'T GET THIS F^#*&^ING PIECE OF S$^* TO COMPILE ON M$VC

Entweder sind sie schlecht gelaunt oder sie haben nicht genug Erfahrung, um zu erfahren, dass Rückschläge bei der Programmierung unvermeidlich sind.

Ich möchte nicht mit solchen Leuten arbeiten.

1
Rei Miyasaka

Dies ist ein etwas geringfügiges Symptom im Vergleich zu den Dingen, die andere aufgelistet haben, aber ich bin ein Python Programmierer und ich sehe dies oft in unserer Codebasis:

try:
    do_something()
except:
    pass

Was mir normalerweise signalisiert, dass der Programmierer nicht wirklich wusste (oder sich nicht darum kümmerte), warum do_something könnte fehlschlagen (oder was es tut) - er hat einfach weiter "herumgespielt", bis der Code nicht mehr abstürzte.


Für diejenigen, die einen Java-ähnlichen Hintergrund haben, ist dieser Block das Python Äquivalent von

try {
    doSomething();
} catch (Exception e) {
    // Don't do anything. Don't even log the error.
}

Die Sache ist, Python verwendet Ausnahmen für "nicht außergewöhnlichen" Code, wie z. B. Tastaturunterbrechungen oder das Ausbrechen einer for-Schleife.

1
mipadi

getter überall lassen mich ausflippen.

und eine besondere Sache: Getter, die an andere Getter delegieren.

das ist schlecht, weil es bedeutet, dass die Person, die geschrieben hat, die Grundlagen der objektorientierten, dh der Kapselung, nicht versteht. Das heißt, wo sich die Daten befinden, sollten die Methoden sein, die auf diese Daten einwirken.

delegation ist für eine Methode nicht alle Getter. das Prinzip ist "erzählen, nicht fragen"; Sagen Sie einem Objekt eine Sache, bitten Sie es nicht um tausend Dinge und tun Sie es dann selbst.

getter machen mich fertig, weil es bedeutet, dass andere oop-Prinzipien hart verletzt werden.

0
Belun

Fehlende Typinformationen.

Schauen Sie sich diese Methodensignaturen an:

  1. public List<Invoice> GetInvoices(Customer c, Date d1, Date d2)

  2. public GetInvoices(c, d1, d2)

In (1) gibt es Klarheit. Sie wissen genau, mit welchen Parametern Sie die Funktion aufrufen müssen, und es ist klar, was die Funktion zurückgibt.

In (2) gibt es nur Unsicherheit. Sie haben keine Ahnung, welche Parameter Sie verwenden sollen, und Sie wissen nicht, was die Funktion zurückgibt, wenn überhaupt etwas. Sie sind effektiv gezwungen, einen ineffizienten Versuch-und-Irrtum-Ansatz für die Programmierung zu verwenden.

0
ThomasX