MsgBox wird zwei mal geöffnet

  • VB.NET
  • .NET (FX) 4.5–4.8

Es gibt 13 Antworten in diesem Thema. Der letzte Beitrag () ist von ErfinderDesRades.

    MsgBox wird zwei mal geöffnet

    Hallo

    bin der Willi und bin noch anfänger in sachen Programmieren.

    Mein erstes Thema und erstes Problem lautet wie folgt.

    In folgendem Code geht es darum Werte in eine Text Datei zu Speichern.
    Dabei wird abgefragt ob alle Felder ausgefüllt sind,
    wenn nicht soll eine MsgBox erscheinen.
    Bei klick auf "Ja" erscheint die MsgBox erneut.

    Quellcode: vb.Net

    1. Private Sub btnSpeichern_Click(sender As Object, e As EventArgs) Handles btnSpeichern.Click
    2. 'Try
    3. Dim SaveFileDialog As SaveFileDialog = New SaveFileDialog
    4. SaveFileDialog.InitialDirectory = IO.Path.Combine(Application.StartupPath, "Kontakte")
    5. SaveFileDialog.Filter = "Textdateien (*.txt)|*.txt"
    6. If SaveFileDialog.ShowDialog(Me) = DialogResult.OK Then
    7. For Each tb In Me.Controls.OfType(Of TextBox)()
    8. For Each mtb In Me.Controls.OfType(Of MaskedTextBox)()
    9. For Each cb In Me.Controls.OfType(Of ComboBox)()
    10. If String.IsNullOrEmpty(tb.Text) Or String.IsNullOrEmpty(cb.Text) Or mtb.MaskCompleted = False Then
    11. Select Case MessageBox.Show("Nicht alle Felder sind ausgefüllt ! " & vbCrLf & "Dennoch Speichern ?", "Speichern", MessageBoxButtons.YesNo, MessageBoxIcon.Information, MessageBoxDefaultButton.Button2)
    12. Case Windows.Forms.DialogResult.Yes
    13. Using sw As New StreamWriter(SaveFileDialog.FileName)
    14. sw.WriteLine(Me.txtName.Text)
    15. sw.WriteLine(Me.txtVorname.Text)
    16. sw.WriteLine(Me.txtDatum.Text)
    17. sw.WriteLine(Me.lblBildName.Text)
    18. If rbWeiblich.Checked = True Then
    19. sw.WriteLine(Me.rbWeiblich.Text)
    20. Else
    21. sw.WriteLine(Me.rbMaennlich.Text)
    22. End If
    23. sw.WriteLine(Me.cbName.Text)
    24. sw.Close()
    25. End Using
    26. Case Windows.Forms.DialogResult.No
    27. MsgBox("Abgebrochen")
    28. Exit Sub
    29. End Select
    30. Else
    31. Using sw As New StreamWriter(SaveFileDialog.FileName)
    32. sw.WriteLine(Me.txtName.Text)
    33. sw.WriteLine(Me.txtVorname.Text)
    34. sw.WriteLine(Me.txtDatum.Text)
    35. sw.WriteLine(Me.lblBildName.Text)
    36. If rbWeiblich.Checked = True Then
    37. sw.WriteLine(Me.rbWeiblich.Text)
    38. Else
    39. sw.WriteLine(Me.rbMaennlich.Text)
    40. End If
    41. sw.WriteLine(Me.cbName.Text)
    42. sw.Close()
    43. End Using
    44. End If
    45. Next
    46. Next
    47. Next
    48. Else
    49. Exit Sub
    50. End If
    51. 'Catch ex As Exception
    52. ' MessageBox.Show(ex.Message)
    53. 'End Try
    54. End Sub


    MfG. Willi
    Willkommen im Forum.
    1. Bitte VB-CodeTags nutzen und Leerzeichen entfernen.
    2.:

    VB.NET-Quellcode

    1. For Each tb In Me.Controls.OfType(Of TextBox)()
    2. For Each mtb In Me.Controls.OfType(Of MaskedTextBox)()
    3. For Each cb In Me.Controls.OfType(Of ComboBox)()
    4. If String.IsNullOrEmpty(tb.Text) Or String.IsNullOrEmpty(cb.Text) Or mtb.MaskCompleted = False Then
    5. Select Case MessageBox.Show

    Den Komplettcode kommentier ich erstmal nicht, da ist zuviel komisches Zeugsl drin. Aber im gezeigten Code hast Du 3 ineinander verschachtelte Schleifen, die jeweils ne MessageBox erzeugen könnten. Das dürfte wohl das Problem sein. Allerdings hast Du mehrere MessageBoxen im Code verteilt und noch nicht spezifiziert, welche mehrfach aufploppt.

    ##########

    Falls Du Tips für besseren Code haben möchtest, gib bescheid.
    Dieser Beitrag wurde bereits 5 mal editiert, zuletzt von „VaporiZed“, mal wieder aus Grammatikgründen.

    Aufgrund spontaner Selbsteintrübung sind all meine Glaskugeln beim Hersteller. Lasst mich daher bitte nicht den Spekulatiusbackmodus wechseln.

    Dieser Beitrag wurde bereits 3 mal editiert, zuletzt von „VaporiZed“ ()

    papawt schrieb:

    For Each tb In Me.Controls.OfType(Of TextBox)()
    For Each mtb In Me.Controls.OfType(Of MaskedTextBox)()
    For Each cb In Me.Controls.OfType(Of ComboBox)()

    Das sind drei ineinander geschachtelte Schleifen, in deren Innerem deine MessageBox aufgerufen wird.
    Da sollte es dich nicht wundern, dass die mehrfach erscheint.
    Anstatt der MessageBox solltest du dir ein Flag setzen und die Abfrage und das Speichern ausserhalb der Schleifen erledigen.
    --
    If Not Program.isWorking Then Code.Debug Else Code.DoNotTouch
    --
    @papawt Ebenfalls Willkommen im Forum. :thumbup:
    Überzeuge Dich davon, dass Dein Code das macht, was Du glaubst, was er machen soll.
    Arbeite ihn Zeile für Zeile durch und vergleiche Ist-Zustand mit Soll-Zustand.
    Hast Du eine Abweichung gefunden, so hast Du einen Fehler gefunden. 8o
    Lerne, Dein Programm zu debuggen: Debuggen, Fehler finden und beseitigen
    Falls Du diesen Code kopierst, achte auf die C&P-Bremse.
    Jede einzelne Zeile Deines Programms, die Du nicht explizit getestet hast, ist falsch :!:
    Ein guter .NET-Snippetkonverter (der ist verfügbar).
    Programmierfragen über PN / Konversation werden ignoriert!
    Hallo
    Danke erst einmal das ihr euch mit meinem Problem befasst.

    Allerdings hast Du mehrere MessageBoxen im Code verteilt und noch nicht spezifiziert, welche mehrfach aufploppt.

    Die MessageBox mit Select Case.
    Ja ich möchte gerne Tips für besseren Code.

    Anstatt der MessageBox solltest du dir ein Flag setzen und die Abfrage und das Speichern ausserhalb der Schleifen erledigen

    Mit Flag hatte ich noch nie etwas zu tun,
    daher weis ich auch nicht wie ich sie anwenden könnte.

    MfG. Willi

    papawt schrieb:

    Mit Flag hatte ich noch nie etwas zu tun,

    VB.NET-Quellcode

    1. Dim Flag As Boolean = False
    2. Flag = True ' statt MessageBox.Show("")
    3. ' und unten, nach der letzten Schleife:
    4. If Flag Then
    5. MessageBox.Show("Meine Meldung")
    6. End If
    Und:
    Befass Dich auf jenen Fall mal mit dem Debuggen: Debuggen, Fehler finden und beseitigen
    Falls Du diesen Code kopierst, achte auf die C&P-Bremse.
    Jede einzelne Zeile Deines Programms, die Du nicht explizit getestet hast, ist falsch :!:
    Ein guter .NET-Snippetkonverter (der ist verfügbar).
    Programmierfragen über PN / Konversation werden ignoriert!
    Hier erstmal ohne Leerzeilen und mit VB-Tags:
    Spoiler anzeigen

    VB.NET-Quellcode

    1. Private Sub btnSpeichern_Click(sender As Object, e As EventArgs) Handles btnSpeichern.Click
    2. 'Try
    3. Dim SaveFileDialog As SaveFileDialog = New SaveFileDialog
    4. SaveFileDialog.InitialDirectory = IO.Path.Combine(Application.StartupPath, "Kontakte")
    5. SaveFileDialog.Filter = "Textdateien (*.txt)|*.txt"
    6. If SaveFileDialog.ShowDialog(Me) = DialogResult.OK Then
    7. For Each tb In Me.Controls.OfType(Of TextBox)()
    8. For Each mtb In Me.Controls.OfType(Of MaskedTextBox)()
    9. For Each cb In Me.Controls.OfType(Of ComboBox)()
    10. If String.IsNullOrEmpty(tb.Text) Or String.IsNullOrEmpty(cb.Text) Or mtb.MaskCompleted = False Then
    11. Select Case MessageBox.Show("Nicht alle Felder sind ausgefüllt ! " & vbCrLf & "Dennoch Speichern ?", "Speichern", MessageBoxButtons.YesNo, MessageBoxIcon.Information, MessageBoxDefaultButton.Button2)
    12. Case Windows.Forms.DialogResult.Yes
    13. Using sw As New StreamWriter(SaveFileDialog.FileName)
    14. sw.WriteLine(Me.txtName.Text)
    15. sw.WriteLine(Me.txtVorname.Text)
    16. sw.WriteLine(Me.txtDatum.Text)
    17. sw.WriteLine(Me.lblBildName.Text)
    18. If rbWeiblich.Checked = True Then
    19. sw.WriteLine(Me.rbWeiblich.Text)
    20. Else
    21. sw.WriteLine(Me.rbMaennlich.Text)
    22. End If
    23. sw.WriteLine(Me.cbName.Text)
    24. sw.Close()
    25. End Using
    26. Case Windows.Forms.DialogResult.No
    27. MsgBox("Abgebrochen")
    28. Exit Sub
    29. End Select
    30. Else
    31. Using sw As New StreamWriter(SaveFileDialog.FileName)
    32. sw.WriteLine(Me.txtName.Text)
    33. sw.WriteLine(Me.txtVorname.Text)
    34. sw.WriteLine(Me.txtDatum.Text)
    35. sw.WriteLine(Me.lblBildName.Text)
    36. If rbWeiblich.Checked = True Then
    37. sw.WriteLine(Me.rbWeiblich.Text)
    38. Else
    39. sw.WriteLine(Me.rbMaennlich.Text)
    40. End If
    41. sw.WriteLine(Me.cbName.Text)
    42. sw.Close()
    43. End Using
    44. End If
    45. Next
    46. Next
    47. Next
    48. Else
    49. Exit Sub
    50. End If
    51. 'Catch ex As Exception
    52. ' MessageBox.Show(ex.Message)
    53. 'End Try
    54. End Sub


    Ich bin kein Fan von

    VB.NET-Quellcode

    1. If Bedingung Then
    2. '100 Codezeilen
    3. Else
    4. Exit Sub
    5. End If

    Stattdessen schreibe ich lieber Abbruchbedingungen nahe am Anfang, also bei Dir:

    VB.NET-Quellcode

    1. If SaveFileDialog.ShowDialog(Me) <> DialogResult.OK Then Exit Sub
    Wie sehen die anderen das?
    Zum Nachfrageflag brauch ich wohl nichts mehr zu sagen.
    Die Speichermethoden in den Zeilen#13-25 und #31-43 sind identisch -> in ne eigene Funktion damit.
    Es freut mich zu sehen, dass Du Using verwendest und das Try-Catch-Konstrukt auskommentiert bleibt. Möge es so bleiben - solange die gefangenen Exceptions nicht spezifisch sind.

    Auch das mit dem Select Case sehe ich hier als deplatziert an - aber das ist Geschmackssache. Du hast nur 2 Fälle: Yes und No. Daher das gleiche Spielchen: Bei No: raus aus der Prozedur. Der nachfolgende Code ergibt sich dann eh nur bei Yes.

    Und da nicht mehr im Code steht, kann ich auch nicht mehr dazu sagen. Als CCD-Fan könnte man alles noch in sinnvolle Funktionen auslagern und die Aufgabe so in einzelne, benannte Teile teilen. Aber auch das ist Geschmackssache.
    Dieser Beitrag wurde bereits 5 mal editiert, zuletzt von „VaporiZed“, mal wieder aus Grammatikgründen.

    Aufgrund spontaner Selbsteintrübung sind all meine Glaskugeln beim Hersteller. Lasst mich daher bitte nicht den Spekulatiusbackmodus wechseln.

    VaporiZed schrieb:

    (...) lieber Abbruchbedingungen nahe am Anfang (...) Wie sehen die anderen das?
    Bin ich bei dir. Mach ich auch so. Gibt nichts schlimmeres, als am ende 100 mal "End If" stehen zu haben und haufenweise Bedingungen ineinander zu verschachteln... Dazu noch mit AndAlso / OrElse arbeiten und man bekommt ne sehr übersichtliche Bedingung hin mit "flacher" Struktur.
    "Gib einem Mann einen Fisch und du ernährst ihn für einen Tag. Lehre einen Mann zu fischen und du ernährst ihn für sein Leben."

    Wie debugge ich richtig? => Debuggen, Fehler finden und beseitigen
    Wie man VisualStudio nutzt? => VisualStudio richtig nutzen
    Guten Morgen
    Hab versucht eure Tips umzusetzen

    VB.NET-Quellcode

    1. Sub Speichern()
    2. Using sw As New StreamWriter(SaveFileDialog.FileName)
    3. sw.WriteLine(Me.txtName.Text)
    4. sw.WriteLine(Me.txtVorname.Text)
    5. sw.WriteLine(Me.txtDatum.Text)
    6. sw.WriteLine(Me.lblBildName.Text)
    7. If rbWeiblich.Checked = True Then
    8. sw.WriteLine(Me.rbWeiblich.Text)
    9. Else
    10. sw.WriteLine(Me.rbMaennlich.Text)
    11. End If
    12. sw.WriteLine(Me.cbName.Text)
    13. sw.Close()
    14. End Using
    15. End Sub


    VB.NET-Quellcode

    1. Private Sub btnSpeichern_Click(sender As Object, e As EventArgs) Handles btnSpeichern.Click
    2. SaveFileDialog.InitialDirectory = IO.Path.Combine(Application.StartupPath, "Kontakte")
    3. SaveFileDialog.Filter = "Textdateien (*.txt)|*.txt"
    4. If SaveFileDialog.ShowDialog(Me) <> DialogResult.OK Then Exit Sub
    5. For Each tb In Me.Controls.OfType(Of TextBox)()
    6. For Each mtb In Me.Controls.OfType(Of MaskedTextBox)()
    7. For Each cb In Me.Controls.OfType(Of ComboBox)()
    8. If String.IsNullOrEmpty(tb.Text) Or String.IsNullOrEmpty(cb.Text) Or mtb.MaskCompleted = False Then
    9. Flag = True
    10. End If
    11. Next
    12. Next
    13. Next
    14. If Flag Then
    15. If MessageBox.Show("Nicht alle Felder sind ausgefüllt ! " & vbCrLf & "Dennoch Speichern ?", "Speichern", MessageBoxButtons.YesNo, MessageBoxIcon.Information, MessageBoxDefaultButton.Button2) <> Windows.Forms.DialogResult.Yes Then Exit Sub
    16. Speichern()
    17. Else
    18. Speichern()
    19. End If
    20. End Sub


    MfG Willi

    petaod schrieb:

    Anstatt der MessageBox solltest du dir ein Flag setzen und die Abfrage und das Speichern ausserhalb der Schleifen erledigen.


    Eigentlich müsste er doch nur die Controls durchgehen, und sofern eine seiner Bedingungen auftritt, die Messagebox zur Abfrage schmeißen und kann danach die Loop beenden, oder nicht?

    Sprich sowas in der Art (ungetestet):

    VB.NET-Quellcode

    1. ​For Each ctrl In Me.Controls
    2. If ((TypeOf ctrl Is Textbox Or TypeOf ctrl Is Combobox) And String.IsNullOrEmpty(ctrl.Text)) Or (TypeOf ctrl Is MaskedTextBox And mtb.MaskCompleted = False) THEN
    3. If MessageBox.Show("Nicht alle Felder sind ausgefüllt ! " & vbCrLf & "Dennoch Speichern ?", "Speichern", MessageBoxButtons.YesNo, MessageBoxIcon.Information, MessageBoxDefaultButton.Button2) <> Windows.Forms.DialogResult.Yes Then Exit Sub
    4. Exit For
    5. End If
    6. Next
    7. Speichern()


    LG, Acr0most
    Wenn das Leben wirklich nur aus Nullen und Einsen besteht, dann laufen sicherlich genügen Nullen frei herum. :D
    Signature-Move 8o
    kein Problem mit privaten Konversationen zu Thema XY :thumbup:
    @Acr0most Jou.
    @papawt Nicht n verschachtelte Schleifen, sondern n einfache Schleifen über je eine Sorte Controls.
    Und wenn Du das unausgefüllte Control hast, kannst Du ja gleich die Hintergrundfarbe auf gelb oder so setzen.
    Natürlich im Erfolgsfalle wieder auf Standard.
    Falls Du diesen Code kopierst, achte auf die C&P-Bremse.
    Jede einzelne Zeile Deines Programms, die Du nicht explizit getestet hast, ist falsch :!:
    Ein guter .NET-Snippetkonverter (der ist verfügbar).
    Programmierfragen über PN / Konversation werden ignoriert!
    Den SaveFileDialog solltest du aber nicht bei jedem Speichern-Click erneut konfigurieren.
    Sowas macht man einmalig im Form_Load (den Filter kannste sogar im Designer festlegen).
    Hat den Useability-Vorteil, dass wenn der User einen anneren Ordner aufsucht, dass dieser neue Ort beim nächsten Speichern nicht weg ist,