Verschachtelte IF-Anweisung

[drei1padsvb]

Hallo zusammen,

ich stehe seit Stunden komplett auf dem Schlauch. Irgendwie seh ich den Wald vor lauter Bäumen nicht mehr.

Bevor ich meinen Code poste, erkläre ich kurz, worum es genau geht.

Wie auf dem Bild zu sehen ist, kann man in dieser Maske Flüge anlegen.
Beim Klick auf Speichern müssen gewisse Dinge geprüft werden:
- Alle ComboBoxes müssen ausgewählt sein (Also nicht auf "Bitte auswählen")
- Abflughafen und Zielflughafen dürfen nicht gleich sein
- Ist der Abflugzeitpunkt (Auswahl zwischen Montag - Sonntag) gleich dem Ankunftszeitpunkt, darf die Abfluguhrzeit nicht vor der Ankunftsuhrzeit liegen.

Da ich die Werte des Spinners als String (HH:mm) zurückgegeben werden, prüfe ich nach der Trennung und zu int-Umwandlug der Zeit:
- Ob die Abflugstunde gleich der Ankunftsstunde ist.
- Sind sie gleich, wird geprüft, ob die Abflugminuten <= Ankunftsminuten.

Sind all diese Dinge nicht erfüllt, kann der Flug angelegt werden.

Hier mein Code dazu (befindet sich in einem ActionListener):

    button_Speichern.addActionListener(new ActionListener() {
    public void actionPerformed(ActionEvent e) {
        if  ((ComboBox1.getSelectedItem()=="Bitte auswählen: ") ||
            (ComboBox2.getSelectedItem()=="Bitte auswählen: ") ||
            (ComboBox3.getSelectedItem()=="Bitte auswählen: ") ||
            (ComboBox4.getSelectedItem()=="Bitte auswählen: ") ||
            (ComboBox5.getSelectedItem()=="Bitte auswählen: ")) 
        
            JOptionPane.showMessageDialog(null, "Bitte alle Felder ausfüllen!");
        
        else
            if (ComboBox3.getSelectedItem()==ComboBox4.getSelectedItem())

            JOptionPane.showMessageDialog(null, "Start- und Zielflughafen gleich!");

            else
                if (ComboBox1.getSelectedItem()==ComboBox2.getSelectedItem())

                    if (Integer.parseInt(new SimpleDateFormat( "HH" ).format( ankunftsSpinner.getDate()))
                         <Integer.parseInt(new SimpleDateFormat( "HH" ).format( abflugSpinner.getDate())))

                        JOptionPane.showMessageDialog(null, "Ankunftszeitpunkt darf nicht vor Abflugzeitpunkt liegen!");
                    else
                        if (Integer.parseInt(new SimpleDateFormat( "HH" ).format( ankunftsSpinner.getDate()))
                            ==Integer.parseInt(new SimpleDateFormat( "HH" ).format( abflugSpinner.getDate())))

                            if (Integer.parseInt(new SimpleDateFormat( "mm" ).format( ankunftsSpinner.getDate()))
                                <Integer.parseInt(new SimpleDateFormat( "mm" ).format( abflugSpinner.getDate())))

                                JOptionPane.showMessageDialog(null, "Ankunftszeitpunkt darf nicht vor Abflugzeitpunkt liegen!");
                else
                JOptionPane.showMessageDialog(null, "Flug angelegt");
                System.out.println("Abflugzeitpunkt: " + ComboBox1.getSelectedItem() + " "
                                + new SimpleDateFormat( "HH:mm" ).format( abflugSpinner.getDate())+ " Uhr");
                System.out.println("Ankunftstpunkt: " + ComboBox2.getSelectedItem() + " "
                                + new SimpleDateFormat( "HH:mm" ).format( ankunftsSpinner.getDate()) + " Uhr");
                System.out.println("Abflughafen: " + ComboBox3.getSelectedItem());
                System.out.println("Zielflughafen: " + ComboBox4.getSelectedItem());
                System.out.println("Fluglinie: " + ComboBox5.getSelectedItem());
    }});

Vielen Dank schonmal!

 

[SlaterB]

du hast eine Frage vergessen,
aber vorerst schon Tipp: schreibe NIEMALS ein if oder else ohne Klammern { },
bzw. du kannst das dir ja gerne sparen, frag dann aber nicht andere nach Fehlern

wer sagt dir, dass das else in Zeile 31 zum if von Zeile 17 gehört und nicht zu den anderen ifs, etwa von Zeile 24 oder 27?
Einrückung ist bedeutungslos


edit:
ach und noch mehr Tipps:
String mit equals vergleichen, nicht ==,

bei vielen Abbruchbedingungen ist vielleicht auch ein return im if nicht schlecht,
dann kann es danach normal weitergehen, muss nicht alles mit else verschachtelt werden

Variablen klein schreiben,

Hilfsvariablen wie
> String c5 = comboBox5.getSelectedItem();
machen den Code kleiner

das Parsen der Date-Bestandteile könnte in Untermethoden oder zumindest in Variablen nicht wiederholt werden

 

[discipulo]

Ohne { } ist dein Code nicht wirklich sauber zu lesen. Dann kommt manchmal er hier ???:L
Ansonsten Zeile 23 bis 30 könnte man glaube auch mit if( bla bla && bal bal) abfragen
dann hast du das schonmal weg, weil meineserachtens nach da beide Bedinungen erfüllt werden müssen.

 

[Dow Jones]

Solche If-Orgien lassen bisweilen übrigens auch übersichtlicher gestalten wenn man Flags/Variablen einführt, die den Stand der Dinge zwischenspeichern. So in der Art von:

   boolean settingsValid = true;

   if( ! alleComboboxenAusgewählt() ) {
      settingsValid = false;
   }

   if( abflughafen == zielflughafen ) {
      settingsValid = false;
   }

   if( abflugzeitpunkt >= ankunftzeitpunkt ) {
      settingsValid = false;
   }

   ...

   if( settingsValid ) {
      abInDenUrlaub();     // =)
   } else {
      anwenderBeschimpfen();
   }

 

[Marco13]

Hab' nicht alles gelesen, aber mal ganz abgesehen von den Hinweisen zum 'if' an sich: Solange man nicht auf "Speichern" klicken können soll, sollte der Button mit
speichernButton.setEnabled(false);
disabled sein, und erst wenn alles OK ist, mit
speichernButton.setEnabled(true);
überhaupt erlauben, da drauf zu klicken.

 

[drei1padsvb]

Vielen Dank für eure Antworten und Hinweise.
Ich habe mir jetzt mal das Dangling Else - Problem genauer angeschaut und die nötigen {} hinzugefügt.

Es klappt fast schon so, wie ich es will.
Nur eine Sache stimmt noch nicht:
Wenn der Abflug- und Ankunftstag gleich sind, wird korrekterweise geprüft, ob die Ankunftszeit gleich, bzw. vor der Abflugzeit liegt.
Die entsprechende Fehlermeldung wird auch angezeigt.
Allerdings wird der letzte else-Block trotzdem ausgeführt.

Das liegt sicherlich an den {}, aber mir raucht grad so der Kopf, dass ich das grad nicht hinkriege.
Naja und ich bin noch Anfänger

    button_Speichern.addActionListener(new ActionListener() {
    public void actionPerformed(ActionEvent e){
        if ((ComboBox1.getSelectedItem().equals("Bitte auswählen: ")) ||
            (ComboBox2.getSelectedItem().equals("Bitte auswählen: ")) ||
            (ComboBox3.getSelectedItem().equals("Bitte auswählen: ")) ||
            (ComboBox4.getSelectedItem().equals("Bitte auswählen: ")) ||
            (ComboBox5.getSelectedItem().equals("Bitte auswählen: ")))

        {

            JOptionPane.showMessageDialog(null, "Bitte alle Felder ausfüllen!");
        }
        else {
            if ((ComboBox3.getSelectedItem()==ComboBox4.getSelectedItem()))

        {
            JOptionPane.showMessageDialog(null, "Start- und Zielflughafen gleich!");
        }
            else {
                if (ComboBox1.getSelectedItem().equals(ComboBox2.getSelectedItem())) {

                    if (Integer.parseInt(new SimpleDateFormat( "HH" ).format( ankunftsSpinner.getDate()))
                         <Integer.parseInt(new SimpleDateFormat( "HH" ).format( abflugSpinner.getDate())))

                        JOptionPane.showMessageDialog(null, "Ankunftszeitpunkt darf nicht vor Abflugzeitpunkt liegen!");
                
                    else
                        if (Integer.parseInt(new SimpleDateFormat( "HH" ).format( ankunftsSpinner.getDate()))
                            ==Integer.parseInt(new SimpleDateFormat( "HH" ).format( abflugSpinner.getDate())) &&

                             Integer.parseInt(new SimpleDateFormat( "mm" ).format( ankunftsSpinner.getDate()))
                             <=Integer.parseInt(new SimpleDateFormat( "mm" ).format( abflugSpinner.getDate())))

                                JOptionPane.showMessageDialog(null, "Ankunftszeitpunkt darf nicht vor Abflugzeitpunkt liegen!");
                
                }else
                System.out.println("Abflugzeitpunkt: " + ComboBox1.getSelectedItem() + " "
                                + new SimpleDateFormat( "HH:mm" ).format( abflugSpinner.getDate())+ " Uhr");
                System.out.println("Ankunftstpunkt: " + ComboBox2.getSelectedItem() + " "
                                + new SimpleDateFormat( "HH:mm" ).format( ankunftsSpinner.getDate()) + " Uhr");
                System.out.println("Abflughafen: " + ComboBox3.getSelectedItem());
                System.out.println("Zielflughafen: " + ComboBox4.getSelectedItem());
                System.out.println("Fluglinie: " + ComboBox5.getSelectedItem());
                JOptionPane.showMessageDialog(null, "Flug angelegt");
    }}}});

 

[SlaterB]

lass deinen Code von einer IDE wie Eclipse formatieren, dann siehst du was zusammengehört,
ohne Plan Klammern zu setzen bringt natürlich auch niemanden voran..

das letzte else bezieht sich ohne Klammern nur auf einen Befehl danach, so soll das bestimmt nicht sein

		button_Speichern.addActionListener(new ActionListener() {
			public void actionPerformed(ActionEvent e) {
				if ((ComboBox1.getSelectedItem().equals("Bitte auswählen: "))
						|| (ComboBox2.getSelectedItem().equals("Bitte auswählen: "))
						|| (ComboBox3.getSelectedItem().equals("Bitte auswählen: "))
						|| (ComboBox4.getSelectedItem().equals("Bitte auswählen: "))
						|| (ComboBox5.getSelectedItem().equals("Bitte auswählen: ")))

				{

					JOptionPane.showMessageDialog(null, "Bitte alle Felder ausfüllen!");
				} else {
					if ((ComboBox3.getSelectedItem() == ComboBox4.getSelectedItem()))

					{
						JOptionPane.showMessageDialog(null, "Start- und Zielflughafen gleich!");
					} else {
						if (ComboBox1.getSelectedItem().equals(ComboBox2.getSelectedItem())) {

							if (Integer.parseInt(new SimpleDateFormat("HH").format(ankunftsSpinner.getDate())) < Integer
									.parseInt(new SimpleDateFormat("HH").format(abflugSpinner.getDate())))

								JOptionPane.showMessageDialog(null, "Ankunftszeitpunkt darf nicht vor Abflugzeitpunkt liegen!");

							else if (Integer.parseInt(new SimpleDateFormat("HH").format(ankunftsSpinner.getDate())) == Integer
									.parseInt(new SimpleDateFormat("HH").format(abflugSpinner.getDate()))
									&&

									Integer.parseInt(new SimpleDateFormat("mm").format(ankunftsSpinner.getDate())) <= Integer
											.parseInt(new SimpleDateFormat("mm").format(abflugSpinner.getDate())))

								JOptionPane.showMessageDialog(null, "Ankunftszeitpunkt darf nicht vor Abflugzeitpunkt liegen!");

						} else
							System.out.println("Abflugzeitpunkt: " + ComboBox1.getSelectedItem() + " "
									+ new SimpleDateFormat("HH:mm").format(abflugSpinner.getDate()) + " Uhr");
						System.out.println("Ankunftstpunkt: " + ComboBox2.getSelectedItem() + " "
								+ new SimpleDateFormat("HH:mm").format(ankunftsSpinner.getDate()) + " Uhr");
						System.out.println("Abflughafen: " + ComboBox3.getSelectedItem());
						System.out.println("Zielflughafen: " + ComboBox4.getSelectedItem());
						System.out.println("Fluglinie: " + ComboBox5.getSelectedItem());
						JOptionPane.showMessageDialog(null, "Flug angelegt");
					}
				}
			}
		});

 

[drei1padsvb]

Ok,
vielen Dank für den Hinweis.
Werd morgen mal schauen, ob ich es hinbekomme.
Heute habe ich keinen Kopf mehr dafür

 

[Marco13]

Bis dahin: Wenn du solchen Code schreibst... denkst du dann nicht selbst manchmal: Hey, MUSS das so kompliziert sein? Vom Wasser auf die Mühlen derer, die behaupten, Java sei langsam, mal abgesehen: Das ist doch ein Krampf :autsch: Schau dir nochmal SlaterB's ratschläge von oben an, und schau' ob du das (WENN du schon den Button enabled läßt, obwohl man nicht draufklicken darf) nicht ein bißchen ähnlicher zu folgendem (ungetesteten!) Code kriegst...

public void actionPerformed(ActionEvent e) {
                if ((ComboBox1.getSelectedItem().equals("Bitte auswählen: "))
                        || (ComboBox2.getSelectedItem().equals("Bitte auswählen: "))
                        || (ComboBox3.getSelectedItem().equals("Bitte auswählen: "))
                        || (ComboBox4.getSelectedItem().equals("Bitte auswählen: "))
                        || (ComboBox5.getSelectedItem().equals("Bitte auswählen: ")))
                {
                    JOptionPane.showMessageDialog(null, "Bitte alle Felder ausfüllen!");
                    return;
                }

                String sourceAirport = ComboBox3.getSelectedItem();
                String targetAirport = ComboBox4.getSelectedItem();
                if (sourceAirport.equals(targetAirport))
                {
                    JOptionPane.showMessageDialog(null, "Start- und Zielflughafen gleich!");
                    return;
                } 

                Date departureDate = abflugSpinner.getDate();
                Date arrivalDate = ankunftsSpinner.getDate();

                if (!arrivalData.after(departureDate))
                {
                    JOptionPane.showMessageDialog(null, "Ankunftszeitpunkt darf nicht vor Abflugzeitpunkt liegen!");
                    return;
                } 

                // Die ganzen System.out's hier, mit den übersichtlichen Variablen...
                JOptionPane.showMessageDialog(null, "Flug angelegt");

 

[drei1padsvb]

Erstmal vielen Dank für deine Antwort und den Lösungsvorschlag.
So hat es wunderbar funktioniert.

Und ich muss sagen, dass es für mich als Anfänger noch nicht so einfach ist, einige Dinge oder Problemstellungen so übersichtlich sehen und angehen zu können, wie eine erfahrener Java-Programmierer, wie du.

Aber das wird nach und nach schon werden

 

[Marco13]

Das hat nur zu einem kleinen Teil mit Erfahrung zu tun. Selbst wenn man nicht auf die Idee kommt, die Dates mit den dafür gemachten Methoden nach größer/kleiner zu vergleichen: Wenn man zwei mal kurz hintereinander so einen unüberscihtlichen Teil wie

(Integer.parseInt(new SimpleDateFormat("HH").format(ankunftsSpinner.getDate()))

hat, könnte man auf die Idee kommen, dort einmal

int arrivalHour = (Integer.parseInt(new SimpleDateFormat("HH").format(ankunftsSpinner.getDate()));

zu machen, und dann den kurzen, sprechenden (!) Variablennamen zu verwenden. Bedenke immer: Code wird höchstens einmal geschrieben, aber potentiell hunderte Male gelesen.