Dein UDF-Header sieht ein wenig merkwürdig aus, es wäre schöner wenn du ihn einheitlich gestaltest. Du verwendest nämlich die Standardmethode und dann noch was von dir.
Statt den Author oben reinzuschreiben gehört er eigentlich in das Author-Feld.
Aber das sind ja keine funktionalen Sachen, also gehen wir mal dahin:
Dein UDF zeigt MsgBoxen mit einem Timeout an wenn ein Fehler passiert ist, aber du sagst nirgends in deiner UDF oder bietest dem Programmierer die Möglichkeit den Timeout anzupassen ggf. wegzulassen.
Das kann eventuell zu Inkonsistenzen im Programmworkflow führen, wenn der Programmierer davon ausgeht, dass der User die Nachricht wegklickt aber sie von alleine verschwindet etc.
Local $ErrorHandle1 = 1
Local $ErrorHandle2 = 1
Local $ErrorHandle3 = 1
Local $ErrorHandle4 = 1
if $Button_2 = "" Then Local $ErrorHandle1 = 0
if $Button_3 = "" Then Local $ErrorHandle2 = 0
if $Button_4 = "" Then Local $ErrorHandle3 = 0
if $Button_5 = "" Then Local $ErrorHandle4 = 0
local $ErrorHandle = $ErrorHandle1 & $ErrorHandle2 & $ErrorHandle3 & $ErrorHandle4
Switch $ErrorHandle
case "0000", "1111", "1000", "1100", "1110"
sleep(200)
case Else
MsgBox(0, "ERROR", "You, messed up button order, try again.", 10)
Return "ERROR"
EndSwitch
Alles anzeigen
Deine Abfrage "0000" ist redundant, da dieser Fall niemals eintreten kann (siehe If-Verzweigung vorher).
Das Sleep ist ebenfalls unnötig, du bremst damit nur das Skript aus, wenn du unbedingt das Konstrukt beibehalten willst lass die Zeile frei.
Du returnst bei diesem Fehler "ERROR" aber bei dem Fehler davor war es "Error", das ist ebenfalls nicht gut, standardisiere deine Rückgaben oder verwende gleich SetError.
Damit kannst du einfach 0 zurückgeben und im Error-Code die Fehlermeldung codieren.
Du hättest das ganze schöner in einer For-Schleife zusammenfassen können, etwa so:
Local $aButtons[] = [$Button_2, $Button_3, $Button_4, $Button_5], $bError = False
For $i = 0 To UBound($aButtons) - 2
If $aButtons[$i] = "" Then
For $j = $i + 1 To UBound($aButtons) - 1
If $aButtons[$j] <> "" Then
$bError = True
ExitLoop 2
EndIf
Next
EndIf
Next
Alles anzeigen
Einfach die Texte in ein Array schreiben und durchlaufen, findest du einen leeren Eintrag läufst du das Array ab dem nächsten Eintrag bis zum Ende durch.
Findest du dorte einen Eintrag der nicht leer ist, so ist ein Fehler aufgetreten.
Wieso fasst du die If-Abfragen nicht zusammen? If $Title = Default Or $Title = "" Then $Title = "Title" sieht doch viel schöner, kompakter und kürzer aus?
if not $Button_1 = "" Then
Local $Button_1Left = $Button_Seperator
Local $GUI_Width_Math = $Button_1Left
EndIf
Du vergleichst case-insensitiv $Button_1 mit dem Leerstring und negierst das Ergebnis. Würdest du einen case-sensitiven Vergleich verwenden würde ich das "Not" ja noch verstehen aber so
hättest du lieber If $Button_1 <> "" Then verwenden können, der Ungleichoperator "<>" ist case-insensitiv.
// NACHTRAG: Ich war mir unschlüssig aber habe nochmal nachgeschaut.
Die Operatorrangfolge ist so gestaltet, dass das "Not" am stärksten bindet, d.h. erst wird der Inhalt von $Button_1 negiert (Strings sind grundsätzlich immer True soweit ich weiß, bis auf den Leerstring) und dann verglichen.
Deine Wahrheitstabelle bleibt zwar nach wie vor richtig, aber ich bin mir 100%ig sicher, dass du das nicht wusstest. Strings auf keinen Fall so behandeln!
Setze entweder eine Klammer um dem Vergleich (wenn du die Schreibweise so behalten willst) oder nimm den von mir vorgeschlagenen Ungleichoperator.
Bisher hattest du schön die Konsistenz mit dem Local-Statement beibehalten aber hier fehlts. Technisch gesehen ist es nicht notwendig, da vom Interpreter automatisch der lokale Scope gewählt wird,
aber der Vollständigkeit halber hättest du es hier auch noch hinsetzen können.
Wieso setzt du die Styles nicht direkt beim Erstellen der GUI? Dafür ist der "style"-Parameter da?
if not $Button_1 = "" Then
Local $Button_1Left = $Button_Seperator
Local $GUI_Width_Math = $Button_1Left
EndIf
if not $Button_2 = "" Then
Local $Button_2Left = $Button_Width+$Button_1Left+$Button_Seperator
Local $GUI_Width_Math = $Button_2Left
EndIf
if not $Button_3 = "" Then
Local $Button_3Left = $Button_Width+$Button_2Left+$Button_Seperator
Local $GUI_Width_Math = $Button_3Left
EndIf
if not $Button_4 = "" Then
Local $Button_4Left = $Button_Width+$Button_3Left+$Button_Seperator
Local $GUI_Width_Math = $Button_4Left
EndIf
if not $Button_5 = "" Then
Local $Button_5Left = $Button_Width+$Button_4Left+$Button_Seperator
Local $GUI_Width_Math = $Button_5Left
EndIf
; ............
if not $Button_1 = "" Then
$GUI_Button_1 = GUICtrlCreateButton($Button_1, $Button_1Left, $Button_Top, $Button_Width, $Button_Height, $BS_DEFPUSHBUTTON)
EndIf
if not $Button_2 = "" Then
$GUI_Button_2 = GUICtrlCreateButton($Button_2, $Button_2Left, $Button_Top, $Button_Width, $Button_Height)
EndIf
if not $Button_3 = "" Then
$GUI_Button_3 = GUICtrlCreateButton($Button_3, $Button_3Left, $Button_Top, $Button_Width, $Button_Height)
EndIf
if not $Button_4 = "" Then
$GUI_Button_4 = GUICtrlCreateButton($Button_4, $Button_4Left, $Button_Top, $Button_Width, $Button_Height)
EndIf
if not $Button_5 = "" Then
$GUI_Button_5 = GUICtrlCreateButton($Button_5, $Button_5Left, $Button_Top, $Button_Width, $Button_Height)
EndIf
Alles anzeigen
Hätte alles super in eine For-Schleife gepasst und hättest du den Arraytrick verwendet (den ich bisschen weiter oben in dieser Antwort gepostet hatte), hättest du das hier um einige Zeilen kürzen können.
Betreibe ich mein Skript im OnEventModus, dann wars das mit deiner UDF und ich kann keine Buttons mehr anklicken.
Du musst abfragen in welchem Modus du dich befindest und je nach dem agieren (oder den Modus setzen und wiederherstellen.)
Das sind jetzt so die Dinge die mir beim Überfliegen mal aufgefallen sind. Es wäre schön wenn du die UDF variabler gestalten würdest indem du X Buttons erlaubst und diese alle dynamisch erstellt und angepasst werden.
5 Buttons hardzucoden ist nicht gerade die feine Art.
Das kann auch komplett weg, es existiert nämlich kein Codepfad, der das letzte Return in deiner Funktion noch auslösen könnte.
Zur Funktionalität selbst (bis auf die Dinge dich angeprangert habe) bleibt mir nur eins zu sagen: Läuft wenn die Voraussetzungen gegeben sind.
Das meiste was ich hier bemängle sind eher Stilsachen als Funktionalität, dennoch liest sich ein sauber formatierter Code der bestimmten Richtlinien folgt einfach schöner und ermöglicht
dir den Wiedereinstieg nach ein paar Wochen wesentlich schneller wenn du nicht mehr weißt wo oben und unten ist.
Damit will ich dir nicht einreden du sollst hier das machen was wir wollen sondern dir ein eigenes System überlegen (oder eins adaptieren) und damit arbeiten.
Code einrücken, Variablen gut benennen, einfach ein bisschen Konsistenz reinbringen, dann sieht das ganze viel besser aus!
Der Anfang ist für deinen Kenntnisstand in Ordnung aber es ist noch Luft nach oben.
Also beim nächsten Mal weniger Pillen schmeißen und fleißiger coden. ![]()
Nachtrag beachten!!!