Best Practice Boolesche Zuweisung [geschlossen]

10

In einem Programm, das ich von einem anderen Entwickler übernommen habe, bin ich auf folgende Bedingung gestoßen:

if (obj.Performance <= LOW_PERFORMANCE)
{
    obj.NeedsChange = true;
}
else
{
    obj.NeedsChange = false;
}

Ich glaube, dieser Code ist redundant und hässlich, deshalb habe ich ihn in eine einfache boolesche Zuordnung geändert, die auf einem Vergleich basiert:

obj.NeedsChange = obj.Performance <= LOW_PERFORMANCE;

Als jemand dies sah, bemerkte er, dass meine Änderung zwar funktional korrekt ist, aber möglicherweise jemanden verwirrt, der sie betrachtet. Er glaubt, dass die Verwendung eines ternären Operators diese Zuordnung klarer macht, während ich das Hinzufügen von redundanterem Code nicht mag:

obj.NeedsChange = (obj.Performance <= LOW_PERFORMANCE) ? true : false;

Seine Argumentation ist, dass es sich nicht lohnt, etwas auf die prägnanteste Weise zu tun, wenn ein anderer Entwickler anhalten und genau herausfinden muss, was Sie getan haben.

Die eigentliche Frage hier ist, welche dieser drei Methoden zum Zuweisen eines Werts zum Booleschen Wert am klarsten obj.NeedsChangeund am wartbarsten ist.

Zach Posten
quelle
25
Die dritte Form ist lächerlich; es heißt nur, was in der zweiten Form bereits offensichtlich sein sollte.
Robert Harvey
6
Dies hängt ganz von Ihren persönlichen Vorlieben ab. Wir können etwas anderes vortäuschen, aber weil sie alle funktional gleichwertig sind, läuft es auf Stil hinaus . Sicher, es gibt einen Unterschied in der Lesbarkeit, aber mein "lesbar und transparent" könnte Ihr "stumpf und undurchsichtig" sein
MetaFight
3
@scriptin 5-8 Zeilen v 1 Zeile ist mehr als bevorzugt, der 5-8 Liner ist normalerweise klarer und besser dafür. In diesem einfachen Beispiel bevorzuge ich die 1-Linie, aber im Allgemeinen habe ich zu viele 10-Liner gesehen, die aus Komfortgründen in 1-Liner verschleiert wurden. Angesichts dessen würde ich mich nie über Variante 1 beschweren, es mag nicht schön sein, aber es macht den Job genauso klar und offensichtlich.
Gbjbaanb
4
Die Optionen 1 und 3 sagen zu mir: "Der Autor versteht die boolesche Logik nicht wirklich."
17 von 26
2
Variante 1 kann nützlich sein, wenn Sie häufig einen Haltepunkt festlegen müssen, der vom Wert abhängt.
Ian

Antworten:

39

Ich bevorzuge 2, aber ich könnte eine kleine Anpassung vornehmen:

obj.NeedsChange = ( obj.Performance <= LOW_PERFORMANCE );

Für mich erleichtern die Klammern das Parsen der Zeile und machen auf einen Blick deutlich, dass Sie das Ergebnis eines Vergleichs zuweisen und keine doppelte Zuordnung vornehmen. Ich bin mir nicht sicher, warum das so ist (ich kann mir keine Sprache vorstellen, in der Klammern tatsächlich eine doppelte Zuordnung verhindern würden), aber wenn Sie Ihren Prüfer zufrieden stellen müssen, ist dies möglicherweise ein akzeptabler Kompromiss.

Jacob Raihle
quelle
4
Dies ist die richtige Antwort - obwohl der Code in der Frage korrekt ist, sagt das Hinzufügen der Klammern dem Leser, dass es sich nicht um eine Zuordnung handelt. Wenn Sie den Code schnell durchgesehen haben, erhalten Sie in den Klammern sofort zusätzliche Informationen, die Sie davon abhalten, genauer hinzuschauen, ob der Code so aussehen soll und kein versehentlicher Fehler ist. Stellen Sie sich zum Beispiel vor, die Zeile wäre a = b == c, haben Sie gemeint, einen Bool zuzuweisen, oder haben Sie gemeint, c sowohl b als auch a zuzuweisen.
Gbjbaanb
Klammern würden eine doppelte Zuweisung in Python verhindern. Selbst in Sprachen, in denen eine doppelte Zuweisung nicht verhindert wird, weisen die Klammern definitiv darauf hin, dass es sich um zwei Arten von Operationen handelt.
user2357112 unterstützt Monica
23

Variante 1 ist leicht zu verstehen, aber das ist ihr einziger Vorteil. Ich gehe automatisch davon aus, dass jeder, der so schreibt, nicht wirklich versteht, worum es bei Booleschen geht, und in vielerlei Hinsicht ähnlich kindlichen Code schreiben wird.

Variante 2 würde ich immer schreiben und erwarten zu lesen. Ich denke, dass jeder, der durch diese Redewendung verwirrt ist, kein professioneller Software-Autor sein sollte.

Variante 3 kombiniert die Nachteile von 1 und 2. 'nuff sagte.

Kilian Foth
quelle
Nun, Variante 1 teilt seinen Vorteil mit Variante 2 ...
Deduplicator
1
+1 für kindlichen Code. Ich habe mir jahrelang solchen Code angesehen, mir fehlte einfach das richtige Wort, um ihn zu identifizieren.
Lilienthal
1
Meine erste Annahme mit Code wie Variante 1 ist, dass die beiden Zweige an einem Punkt in der Vergangenheit komplizierter waren und jemand beim Refactoring nicht darauf geachtet hat. Wenn es jedoch so war, als es zum ersten Mal geschrieben wurde, dann stimme ich zu, "Boolesche
Werte
13

Immer wenn Code komplizierter ist, als er sein muss, wird ein "Was soll das tun?" Ausgelöst. Geruch im Leser.

Das erste Beispiel lässt mich beispielsweise fragen: "Gab es irgendwann andere Funktionen in der if / else-Anweisung, die entfernt wurden?"

Beispiel (2) ist einfach, klar und macht genau das, was benötigt wird. Ich lese es und verstehe sofort, was der Code tut.

Der zusätzliche Flaum in (3) würde mich fragen lassen, warum der Autor es so geschrieben hat, anstatt (2). Es sollte einen Grund geben, aber in diesem Fall scheint es keinen zu geben, daher ist es überhaupt nicht hilfreich und schwerer zu lesen, da die Syntax etwas Vorhandenes vorschlägt, das nicht vorhanden ist. Der Versuch zu lernen, was vorhanden ist (wenn nichts vorhanden ist), erschwert das Lesen von Code.

Enderland
quelle
2

Es ist leicht zu erkennen, dass Variante 2 und Variante 1 über eine Reihe offensichtlicher und einfacher Refactorings miteinander verbunden sind:

if (obj.Performance <= LOW_PERFORMANCE)
{
    obj.NeedsChange = true;
}
else
{
    obj.NeedsChange = false;
}

Hier haben wir unnötige Codeduplizierungen, wir können die Zuordnung herausrechnen:

obj.NeedsChange = if (obj.Performance <= LOW_PERFORMANCE)
{
    true
}
else
{
    false
}

oder prägnanter geschrieben:

obj.NeedsChange = if (obj.Performance <= LOW_PERFORMANCE) true else false

Nun sollte es sofort offensichtlich sein, dass dies true zuweist, wenn die Bedingung wahr ist, und false, wenn die Bedingung falsch ist. IOW weist einfach den Wert der Bedingung zu, dh er ist äquivalent zu

obj.NeedsChange = obj.Performance <= LOW_PERFORMANCE

Die Varianten 1 und 3 sind typische Rookie-Codes, die von jemandem geschrieben wurden, der den Rückgabewert eines Vergleichs nicht versteht.

Jörg W Mittag
quelle
Ich würde Ihren if (...) ... falschen Teil als Kommentar kurz vor dem netten hinzufügen, dann erhalten Sie den einfachen Scan durch die Codeklarheit und den besseren Code.
DaveM
2

Während Ihre Programmierung auf explizite über implizite „als ob der Mann, der den Code Aufrechterhaltung endet wird ein heftiger Psychopath sein, der weiß , wo Sie leben“ neigen sollte, Sie können ein paar grundlegende Dinge davon ausgehen , dass Ihre psycho Nachfolger zuständig sein wird.

Eine davon ist die Syntax der Sprache, die er verwendet.

obj.NeedsChange = obj.Performance <= LOW_PERFORMANCE;

ist jedem klar, der die C / C ++ / C # / Java / Javascript-Syntax kennt.

Es ist auch viel besser lesbar als 8 Zeilen.

if (obj.Performance <= LOW_PERFORMANCE)
{
    obj.NeedsChange = true;
}
else
{
    obj.NeedsChange = false;
}

und weniger fehleranfällig

if (obj.Performance <= LOW_PERFORMANCE)
{
    obj.NeedsChange = true;
}
else
{
    obj.Needschange = false;
}

Und es ist besser, als unnötige Zeichen hinzuzufügen, als hätten Sie die Syntax der Sprache halb vergessen:

obj.NeedsChange = obj.Performance <= LOW_PERFORMANCE ? true : false;

obj.NeedsChange = (obj.Performance <= LOW_PERFORMANCE);

Ich denke, die C-ähnliche Syntax vieler Sprachen ist sehr falsch: inkonsistente Reihenfolge der Operationen, inkonsistente Links / Rechts-Assoziativität, überladene Verwendung von Symbolen, doppelte Klammern / Einrückungen, ternäre Operatoren, Infixnotation usw.

Die Lösung besteht jedoch nicht darin, eine eigene Version davon zu erfinden. Auf diese Weise liegt der Wahnsinn, da jeder seinen eigenen erschafft.


Im Allgemeinen ist die Menge , die Real World TM -Code unlesbar macht, die Menge davon.

Zwischen einem nicht pathologischen 200-Zeilen-Programm und einem trivial identischen 1.600-Zeilen-Programm ist das kürzere fast immer leichter zu analysieren und zu verstehen. Ich würde Ihre Änderung jeden Tag begrüßen.

Paul Draper
quelle
1

Die meisten Entwickler werden die 2. Form mit einem Blick verstehen können. Meiner Meinung nach ist eine Vereinfachung wie in der 1. Form einfach unnötig.

Sie können die Lesbarkeit verbessern, indem Sie Leerzeichen und geschweifte Klammern hinzufügen, z.

obj.NeedsChange =    obj.Performance <= LOW_PERFORMANCE;

oder

obj.NeedsChange = ( obj.Performance <= LOW_PERFORMANCE );

wie von Jacob Raihle erwähnt.

Uday Shankar
quelle