Stilfrage: IF-Statements

[FredFerkel]

Hallo,

ich wollte mal eure Meinung zu folgender Frage hören; Was findet ihr besser, bzw. ist allgemein erwünschter:

Lösung1:

boolean passt = false;
while (egalwas){
   if (bedingung1==true){
      if (bedingung2==true){
         if (bedingung3==true){
            passt = true;
            break;
         }
      }
   }
}

Lösung2:

boolean passt = false;
while (egalwas){
   if (bedingung1==false){
      continue;
   }
   if (bedingung2==false){
      continue;
   }
   if (bedingung3==false){
      continue;
   }
   passt=true;
   break;
   }
}

...es geht darum wie man If-Statements verschachtelt. Ich tendiere dazu die 2. Lösung übersichtlicher zu finden. Was meint ihr?

Salut
Fred

 

[DerEisteeTrinker]

Wie wäre es mit

if (bedingung1 && bedingung2 && bedingung3) {
// dein Quellcode
}

 

[FredFerkel]

Wie wäre es mit

if (bedingung1 && bedingung2 && bedingung3) {
// dein Quellcode
}

Ja, klar...

Aber das war nur ein Beispiel, dass ich bewusst einfach gehalten habe. Es könnten komplexere Statements sein, mit weiteren Schritten dazwischen.

 

[DerEisteeTrinker]

Beim Stil gibt es zwar Richtlinien, aber so ein Allheilmittel existiert nicht. Manchmal muss man es verschachteln, manchmal muss man eine riesen Bedingung zusammen wursten. Denk beim Schreiben einfach daran, dass vllt irgendwer anders das mal lesen muss. Ansonsten halte die Dinge so einfach wie möglich.

Mich würde Lösung 2 ankotzen, hab sie auch erst beim 3ten Mal lesen richtig verstanden

 

[FredFerkel]

Mich würde Lösung 2 ankotzen, hab sie auch erst beim 3ten Mal lesen richtig verstanden

Dachte ich mir! Ich finde das sehr übersichtlich, aber das sieht man seltener als Lösung 1.

 

[DerEisteeTrinker]

Das liegt nur daran, dass ich noch nie das Kommando

continue;

benutzt habe. Hab mich immer mit einem

break;

zufrieden gegeben.

Also mein Rat schreib so, wie du es am Besten lesen kannst, aber andere auch noch halbwegs was davon verstehen. Dann kann schon mal nichts schiefgehen

 

[ARadauer]

Also ich finde beide schlecht.
Aber 1 besser als 2

 

[VfL_Freak]

Moin,

mal davon abgesehen, was nun optisch eleganter ist (das ist sicher eine Geschmacksfrage und auch Gewohnheitssache) solltest Du noch bedenken, das Deine Lösung 1 ggf. etwas performanter ist.

Während Du bei Lösung 2 IMMER alle drei Bedingungen prüfst, geschieht dies im, Fall 1 u. U. nicht - bspw. wenn Bedingung 1 == false ist! Hier werden dann die Bedingungen 2 und 3 gar nicht erst geprüft !

Ist sicher bei diesem Beispiel nicht soooo relevant, kann es aber bei großen Programmen, de man optimieren will, schnell werden

Gruß
Klaus

 

[Marco13]

Nee, bei 2. wird auch nur überprüft, bis die erste Bedinung nicht mehr zutrifft, und dann ggf. mit continue wieder an den Schleifenanfang gehüpft.

Eine allgemeine Regel gibt es dazu wohl nicht. Eigentlich sollte man mit "continue" vorsichtig sein, weil es unstrukturiert und ggf. verwirrend und unübersichtlich ist. Aber eine zu tiefe Einrückung auch. Wenn man das nicht durch geeignete Methoden flachdrücken kann oder will, finde ich, dass die 2. Lösung OK ist. Es gibt sicher Fragen in dem Programm, über die man sich mehr Gedanken machen sollte

 

[VfL_Freak]

Moin,

Nee, bei 2. wird auch nur überprüft, bis die erste Bedinung nicht mehr zutrifft, und dann ggf. mit continue wieder an den Schleifenanfang gehüpft

Oops

Klar, Du hast natürlich Recht !
Da war ich wohl ein wenig (Schnee-)blind ..... :lol:

Gruß
Klaus

 

[bygones]

Also ich finde beide schlecht.

mehr gibts net zu sagen....

 

[Marco13]

mehr gibts net zu sagen....

Doch, nämlich wie die Alternative aussehen soll Natürlich war der Codeschnispel nur ein Suggestivbeispiel. Natürlich würde man erstmal schauen, ob man das nicht "sinnvoll" zu sowas machen kann wie

while (something)
{
    if (someMethodCheckingAllThisStuff()) { doit(); }
}

Aber es gibt IMHO Fälle, wo solche "mini-watchdogs", die am Anfang einer Schleife prüfen, ob weitergemacht werden soll, gerechtfertigt sind.

 

[bygones]

Doch, nämlich wie die Alternative aussehen soll Natürlich war der Codeschnispel nur ein Suggestivbeispiel. Natürlich würde man erstmal schauen, ob man das nicht "sinnvoll" zu sowas machen kann wie

while (something)
{
    if (someMethodCheckingAllThisStuff()) { doit(); }
}

Aber es gibt IMHO Fälle, wo solche "mini-watchdogs", die am Anfang einer Schleife prüfen, ob weitergemacht werden soll, gerechtfertigt sind.

allein deine häufige nutzung von "natürlich" zeigt doch dass es besser und sinnvollere wege gibt. das andere ist dann mehr ausrede

 

[Marco13]

Natürlich war die doppelte Verwendung von "Natürlich" Absicht :bae: Und Natürlich sollte die eine gewisse Zustimming implizieren. Aber natürlich gibt es auch Stil-Nazis, die irgendwann mal Alpha list of refactorings überflogen haben, und meinen, dass man pauschal und ohne Nachzudenken jedes
if (a & b) { ... }
zu einem
if (both(a,b)) { ... }
machen sollte, weil das ein schlauer Mensch mal auf einer Seite geschrieben hat (und sie nicht zufällig auf den Link geklickt haben, in dem genau das Gegenteil beschrieben wird).
Viel mehr als dass man sich das von Fall zu Fall überlegen (aber deswegen nicht zu Grabenkämpfen übergehen sollte) habe ich eigentlich nicht gesagt.

 

[tagedieb]

Ich finde die Auslagerung einer komplexen Bedingung in eine eigene Methode sehr uebersichtlich und einfach. Refactoring als Stichwort.

 

[Marco13]

Genau das meinte ich. Und schließlich hängt es davon ab, was "komplex" heißt, ob nicht doch das Gegenteil, Refactoring: Inline Method , angewendet werden sollte

 

[ARadauer]

Und schließlich hängt es davon ab, was "komplex" heißt

drei verschachtelte if, wobei man davon ausgehen kann das in einem realen beispiel zwischen den ifs noch andererer code steht, finde ich komplex

dass man pauschal und ohne Nachzudenken jedes

allgemeiner code = pauschale antwort
konkreter code = genauere analyse...

 

[DEvent]

Wenn du 3 Ifs hast,die so ähnlich sind, dann solltest du den Code durch Objekt Orientierung ersetzen. Entweder durch Interface/Klassen

interface Bedingung {
    boolean passt();
}

class Bedingung1 implements Bedingung
class Bedingung2 implements Bedingung
class Bedingung3 implements Bedingung

bedingunen = new ArrayList 
Bedingung passt;
for (Bedingung b : bedingunen) {
    if (b.passt()) {
        passt = b;
        break;
    }
}

Oder auch durch ein Enum:

enum Bedingungen {

    B1 {
        public boolean passt() {
            // ...
        }
    },
    B2 {
        public boolean passt() {
            // ...
        }
    },
    B3 {
        public boolean passt() {
            // ...
        }
    },

    public abstract boolean passt();

    public static Bedingungen sucheBisPasst() {
        for (Bedingungen b : Bedingungen.values()) {
            if (b.passt()) {
                return b;
            }
        }
    }
}

 

[FredFerkel]

...also Objektorientierung ist ja schön und gut, aber das riecht schon etwas nach einer Erdölraffinerie. Abgesehen davon ist diese Lösung nicht wirklich performanter, oder? Drei verschachtelte IF-Statements sind auch nicht soo furchtbar ungewöhnlich. Ich hab das jedenfalls schön öfter gesehen, auch in OpenSource-Projekten. Solange man Code nicht zweimal verwendet ist es meines Erachtens auch nicht zwingend nötig, ihn in eine Methode auszulagern. Es hängt alles von der Komplexität hab, wie bereits gesagt.

Ich will hier auch keine akademische Diskussion um den raffiniertsten Code lostreten. Wem es besser gefällt, der darf mir auch sagen, welche Lösung schlechter ist.

 

[tfa]

Abgesehen davon ist diese Lösung nicht wirklich performanter, oder?

Na und?

Drei verschachtelte IF-Statements sind auch nicht soo furchtbar ungewöhnlich. Ich hab das jedenfalls schön öfter gesehen, auch in OpenSource-Projekten. Solange man Code nicht zweimal verwendet ist es meines Erachtens auch nicht zwingend nötig, ihn in eine Methode auszulagern.

Aber vielleicht sinnvoll, wenn der Code dadurch verständlicher und übersichtlicher wird. Die Tatsache, dass man sowas wie 3 verschachtelte ifs häufig sieht heißt ja nicht, dass das auch gut ist.
Das Beispiel mit dem Interface bzw. Enum ist ein praktisches Entwurfsmuster. Sicherlich für eine kleine if-Anweisung völlig übertrieben, aber für größere Sachen das Mittel der Wahl.

 

[Gastredner]

Schön, dass wir gerade dieses Thema haben.
Ich arbeite momentan an einem RCP-Projekt, welches auch diverse Dialoge zum Anlegen neuer Entitäten bietet. Diese werden als TitleAreaDialog umgesetzt.
Die Benutzereingaben werden dabei bisher einfach nur von einer einzelnen Methode in Form kaskadierter If-Statements validiert. Zudem setzt die Methode auch die entsprechenden Fehlermeldungen:

/**
 * Validiert die Eingaben und setzt passende Fehlermeldungen.
 * 
 * @return {@code true}, wenn alle Eingaben valide sind. Ansonsten
 *         {@code false}.
 */
private boolean validate() {
	setErrorMessage(null);
	if (!tfName.getText().isEmpty()) {
		if (!tfNumber.getText().isEmpty()) {
			if (!cvType.getSelection().isEmpty()) {
				if (!cvManufacturer.getSelection().isEmpty()) {
					if (!alreadyExists()) {
						return true;
					} else {
						setErrorMessage("Dieses Modell existiert bereits!");
					}
				} else {
					setErrorMessage("Es muss ein Hersteller angegeben werden!");
				}
			} else {
				setErrorMessage("Es muss ein Gerätetyp angegeben werden!");
			}
		} else {
			setErrorMessage("Es muss eine Modellnummer angegeben werden!");
		}
	} else {
		setErrorMessage("Es muss eine Modellbezeichnung angegeben werden!");
	}
	return false;
}

Wie würde sich dies eleganter lösen lassen? Das Vorgehen von DEvent kommt mir hierbei eher unpassend vor, da es einerseits viel überflüssigen Code produziert und zudem die Bedingungen immerzu eine Referenz auf die Widgets bräuchten (oder auf deren Text-String oder das ISelection-Objekt, was sich dann aber nur hässlich über eine passt(Object)-Methode umsetzen ließe, was IMHO aufgrund des dauernden Herumcastens hässlich wäre).
Andererseits ist der derzeitige Weg bereits bei 4 Widgets recht hässlich - und vermutlich vollkommen unpraktisch, wenn es noch mehr Widgets werden sollten.

 

[DEvent]

Kann man von der TextField und der CheckBox Klasse ableiten? Dann würde sich was anbieten:

interface ValidateComponent {

    String validate();
}

class MyValidationTextField extends TextField implements ValidateComponent {

    public String validate() {
        if (getText().isEmpty()) { 
            return String.format("Es muss eine %s angegeben werden!", getName()); 
        } 
        else {
            return null;
        }
    }
}

class MyValidationCheckBox extends CheckBox implements ValidateComponent {

    public String validate() {
        if (getSelection().isEmpty()) { 
            return String.format("Es muss eine %s angegeben werden!", getName()); 
        } 
        else {
            return null;
        }
    }
}

// ...
components = new ArrayList<ValidateComponent>()

private boolean validate() {
    for (ValidateComponent c : components) {
        valid = c.validate();
        setErrorMessage(valid);
    }
    return valid != null;
}

Ansonsten, Validation gehört in ein RCP Framework. Wenn validation nicht in euer RCP Framework eingebaut ist, würde ich mir schnell ein neues Framework suchen.

Sowas gehört mit Annotations im Modell gelöst. Hier ist ein Beispiel dafür: JaValid Core Example 1: Basics

 

[DEvent]

Man kann solche verschachtelte oder aneinander gereihten Ifs immer durch OO Methoden lösen. Die Frage ist nur welche Methode sich anbietet und ob der Overhead sich lohnt.

Ifs sind immer schneller geschrieben, OO Methoden die diese Ifs ersetzen brauchen Zeit und müssen integriert werden. Für 5 Felder würde ich erst mal auch nur Ifs schreiben, wenn ich es aber durch eine OO Methode für jeden Feld in der Anwendung die Ifs ersetzen kann, dann lohnt es sich vielleicht.

 

[tfa]

Ein Validation-Framework einzusetzen wäre natürlich die erste Wahl. Aber grundsätzlich kann man es auch ohne verschachtelte ifs machen:

public boolean validate() {
	ArrayList<Message> msg = new ArrayList<Message>();
	msg.appendAll(checkModell());
	msg.appendAll(checkHersteller());
	msg.appendAll(checkGeraetetyp());
	msg.appendAll(checkModellnummer());
	msg.appendAll(checkModellbezeichnung());
	setMessages(msg);
	return msg.isEmpty();
}

Das hat auch den Vorteil, dass der Anwender gleich über alle Probleme gelichzeitig informiert wird und nicht kleckerweise. Schon ziemlich umständlich, wenn man 10 Eingabefehler hat, 10 mal auf OK klicken zu müssen um dann immer den nächsten Fehler präsentiert zu bekommen.

 

[MasterK]

boolean passt = false;
while (egalwas){
   if (bedingung1==true){
      if (bedingung2==true){
         if (bedingung3==true){
            passt = true;
            break;
         }
      }
   }
}

Waahhh... diesen scheiss mit dem missbrauchen einer while-anweisung für so einen mist hab ich bei nem kollegen vor ner weile auch gesehen. Da dachte ich mir nur "WTF!?!". Das scheint mir so ein "cooles" stück "trick" zu sein, was da grad in mode ist? Tut mir leid, das ist einfach bullshit. Man kann jeden code so strukturieren, dass er ohne diesen while()-break-mist wesentlich besser aussieht und sich kein bischen anders verhält. Beim beispiel da oben zB ist "while(irgendwas)" doch nichts anderes als "if(irgendwas)", nur dass der code natürlich absolut nicht im ersten moment verständlich ist, wenn man nicht weiss, was der, der ihn verbrochen hat, damit bezwecken wollte.

 

[Gastredner]

public boolean validate() {
	ArrayList<Message> msg = new ArrayList<Message>();
	msg.appendAll(checkModell());
	msg.appendAll(checkHersteller());
	msg.appendAll(checkGeraetetyp());
	msg.appendAll(checkModellnummer());
	msg.appendAll(checkModellbezeichnung());
	setMessages(msg);
	return msg.isEmpty();
}

Das hat auch den Vorteil, dass der Anwender gleich über alle Probleme gelichzeitig informiert wird und nicht kleckerweise. Schon ziemlich umständlich, wenn man 10 Eingabefehler hat, 10 mal auf OK klicken zu müssen um dann immer den nächsten Fehler präsentiert zu bekommen.

TitleAreDialog unterstützt allerdings keine mehrfachen Fehlermeldungen. Ich müsste daraus wenn einen einzelnen String basteln und den dann als Fehlermeldung setzen, was allerdings dann wohl in überlangen (und vielleicht gar nicht mehr vernünftig dargestellten) Fehlermeldungen resultieren würde. Daher wäre es relativ sinnlos, alle Meldungen in eine Liste zu stecken. Dazu kommt der überflüssige Overhead (wobei der ja praktisch vernachlässigbar sein sollte).

Vom Text-Widget kann wohl nicht ableiten. Vom ComboViewer vermutlich schon, Text müsste man dann in ein anderes Widget schachteln und dieses dann nutzen.

Aber danke für die Vorschläge.

 

[Marco13]

Waahhh... diesen scheiss mit dem missbrauchen einer while-anweisung für so einen mist hab ich bei nem kollegen vor ner weile auch gesehen. ...

Bei allem, was ich geschrieben habe, bin ich (offenbar irrtümlich ) davon ausgegangen, dass das eine Schleife sein sollte - hatte also statt des 'break' dort ein 'continue' im Kopf. So eine Schleife für sowas zu verwenden ist wirklich nicht schön...

Zu den OO-Ansätzen: ... Das erinnert mich ein bißchen an 99 Bottles of Beer | Language Java ... ???:L Es geht immernoch nur um ein paar if-Abfragen, oder?

 

[MasterK]

Bei allem, was ich geschrieben habe, bin ich (offenbar irrtümlich ) davon ausgegangen, dass das eine Schleife sein sollte - hatte also statt des 'break' dort ein 'continue' im Kopf. So eine Schleife für sowas zu verwenden ist wirklich nicht schön...

Hatte ich beim erwähnten kollegen ebenfalls gedacht. "Du, du hast da ne endlosschleife eingebaut". Bis er mir diesen "tollen trick" erläuterte. Obwohl das im beispiel tatsächlich dazu noch eine potentielle (sinnlose) endlosschleife ist.
Im endeffekt ist das nichts weiter als ein "goto". Nur anders geschrieben. Übel.

 

[maki]

Waahhh... diesen scheiss mit dem missbrauchen einer while-anweisung für so einen mist hab ich bei nem kollegen vor ner weile auch gesehen. Da dachte ich mir nur "WTF!?!". Das scheint mir so ein "cooles" stück "trick" zu sein, was da grad in mode ist? Tut mir leid, das ist einfach bullshit. Man kann jeden code so strukturieren, dass er ohne diesen while()-break-mist wesentlich besser aussieht und sich kein bischen anders verhält. Beim beispiel da oben zB ist "while(irgendwas)" doch nichts anderes als "if(irgendwas)", nur dass der code natürlich absolut nicht im ersten moment verständlich ist, wenn man nicht weiss, was der, der ihn verbrochen hat, damit bezwecken wollte.

Bist du sicher dass es ein Missbrauch ist?
Semantisch sind die beiden Versionen mit while bzw. if nicht gleichwertig. Schleifen eignen sich nämlich gut für Collections zB.