Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ReGaHSS: .StrValueByIndex() -> fehlerhafte String-Ausgabe bei Indexwert -1 #2597

Closed
BadenPower opened this issue Jan 9, 2024 · 9 comments
Closed
Labels
🐛 bug-report Something isn't working 🏷️ ReGaHss This refs the ReGaHss component

Comments

@BadenPower
Copy link

BadenPower commented Jan 9, 2024

Describe the issue you are experiencing

Wird die Methode .StrValueByIndex() mit dem Parameterwert -1 im 2. Parameter aufgerufen, dann wird der gesamte String zurückgegeben.

Describe the behavior you expected

Erwartet würde die Rückgabe eines Leerstrings, so wie es bei allen anderen Indexangaben ausserhalb des gültigen Wertebereiches zutrifft

Steps to reproduce the issue

Ausführung des folgenden Skriptes unter "Skript testen":

WriteLine("Start");
WriteLine(dom.BuildLabel());
string lValueList = "A;B";
WriteLine(lValueList.StrValueByIndex(";",2));
WriteLine(lValueList.StrValueByIndex(";",1));
WriteLine(lValueList.StrValueByIndex(";",0));
WriteLine(lValueList.StrValueByIndex(";",-1));
WriteLine(lValueList.StrValueByIndex(";",-2));
WriteLine("Ende");

What is the version this bug report is based on?

CCU3 mit ReGaHSS-Version R1.00.0388.0235

Which base platform are you running?

rpi3 (RaspberryPi3)

Which HomeMatic/homematicIP radio module are you using?

n/a

Anything in the logs that might be useful for us?

bestehend seit:
unbekannt (bereits mit ReGaHss-Version R1.00.0388.0102)

Additional information

Ausgabe des Skriptes:

Start
R1.00.0388.0235

B
A
A;B

Ende
@BadenPower BadenPower added the 🐛 bug-report Something isn't working label Jan 9, 2024
@jens-maus jens-maus added the 🏷️ ReGaHss This refs the ReGaHss component label Jan 9, 2024
@jens-maus
Copy link
Owner

@BadenPower Danke für den Report

Ich hab mir erlaubt das Beispielskript einmal kurz zu erweitert um den Fall hier etwas deutlicher zu sehen:

WriteLine("Start");
WriteLine(dom.BuildLabel());
string lValueList = "A;B";
WriteLine(" 2:'" # lValueList.StrValueByIndex(";",2) # "'");
WriteLine(" 1:'" # lValueList.StrValueByIndex(";",1) # "'");
WriteLine(" 0:'" # lValueList.StrValueByIndex(";",0) # "'");
WriteLine("-1:'" # lValueList.StrValueByIndex(";",-1)# "'");
WriteLine("-2:'" # lValueList.StrValueByIndex(";",-2)# "'");
WriteLine("-3:'" # lValueList.StrValueByIndex(";",-3)# "'");
WriteLine("Ende");

Das ergibt dann folgenden:

Start
R1.00.0388.0235
 2:''
 1:'B'
 0:'A'
-1:'A;B'
-2:''
-3:''
Ende

In der Tat sieht man das bei -1 einfach der gesamte String zurückgegeben wird. Ich sehe allerdings auch im Quellcode das hier explizit dieser Fall (-1) abgeprüft wird. Könnte mir also vorstellen das dies vielleicht nicht wirklich ein Bug ist weil der ursprüngliche Autor das vielleicht genau so vorgesehen hatte – eben das bei -1 der gesamte String zurückgegeben wird. D.h. es könnte doch sehr wohl ein valides Nutzungsszenario sein das man eben via -1 dann den gesamten string haben möchte? Vielleicht ist das daher eher als Bug der offiziellen Skript-Dokumentation zu werten und man sollte dort hinterlegen das bei -1 immer der gesamte String zurückgegeben wird? Kann es nicht Nutzungzenarien geben wo solch ein Verhalten von Vorteil sein könnte?

@BadenPower
Copy link
Author

BadenPower commented Jan 9, 2024

Es ist schwer vorstellbar, dass es ein Szenario gibt, welches ausgerechnet bei -1 den ganzen String zurückgeben soll.

Man stelle sich den Fall vor, dass die Liste nur einen Wert hat, also das Trennzeichen gar nicht im Ausgangsstring vorhanden ist. Dann würde auch beim Index 0 der gesamte String zurückgegeben, was auch vollkommen in Ordnung ist.

Eine Unterscheidung dieser 2 Fälle wäre dann wiederum nur durch Auswertung des verwendeten Index möglich. Darum ist mir unschlüssig, weshalb man dann nicht im Vorfeld auf den verwendeten Index reagiert und im Falle von -1 erst gar nicht die Mehtode ausführt, sondern gleich den Ausgangsstring verwendet.

Da im Bereich ReGaHSS oftmals eine Differenz von 1 oder -1 zwischen zwei Werten zu Problemen (siehe u.A. .Random()-Bug) führt, könnte ich mir durchaus vorstellen, dass dies eventuell ein eingefügter Workaround zur Vermeidung einer möglichen Exception sein könnte.

@jens-maus jens-maus added the ❓ undecided No decision to accept or reject ticket yet label Jan 10, 2024
@jens-maus
Copy link
Owner

Also ich hab mir das nochmal genauer angeschaut und auch nochmal durch den Kopf gehen lassen. In der offiziellen Dokumentation steht momentan folgendes:

Bildschirmfoto 2024-01-12 um 11 31 43

D.h. ein index < 0 ist quasi nicht spezifiziert. Die Frage wäre also in der tat wie man damit nun umgeht auch im Bezug auf die hier aufgezeigten Unterschiede. Man könnte man natürlich einfach ein ScriptRunTimeError werfen wenn die StrValueByIndex Methode mit einem index < 0 aufgerufen wird. Die Frage wäre nur ob das nicht ggf. bei exitierenden Programmen wo ggf. der index via einer variable verwendet wird zu einem harten Abbruch des Programmes führen würde. Deshalb würde ich dazu gerne nochmal Meinungen hier einsammeln wollen. Oder aber man ändert das in der Tat dahingehend ab das hier für alle Werte < 0 ein leere String zurückgegeben wird? Denkbar wäre natürlich auch bei einem Wert < 0 einfach statt von links nach rechts die suche umzudrehen und von rechts nach links zu suchen und dann den entsprechenden Substring auszugeben. Aber ich denke das wäre ggf. eine Erweiterung der Funktion die vielleicht auch gewisse Probleme bei schon existierenden Programmen geben könnte.

Zu sagen gilt natürlich auch noch das wenn die Funktion nun entsprechend geändert werden sollte das bei < 0 immer ein leer String zurückgegeben wird, dann könnte das genauso missverstanden werden wenn man ein Eingangstring wie z.B. A;B;;D hat und den index auf 2 setzt, denn dort ist ja in der liste ohnehin schon ein leerer String.

@Baxxy13
Copy link
Contributor

Baxxy13 commented Jan 12, 2024

Ich bin für ScriptRunTimeError wenn ich die Funktion außerhalb der dokumentierten Spezifikation benutze.

@jens-maus
Copy link
Owner

Ich bin für ScriptRunTimeError wenn ich die Funktion außerhalb der dokumentierten Spezifikation benutze.

Das würde dann bedeuten: index < 0 => ScriptRunTimeError und bei index > N (d.h. Anzahl der substrings) dann der leere String?

@Baxxy13
Copy link
Contributor

Baxxy13 commented Jan 12, 2024

Ja, wäre für mich ok.

@BadenPower
Copy link
Author

BadenPower commented Jan 12, 2024

In meinen Augen macht es keinen Sinn zu unterscheiden, ob sich der Wert im negativen Bereich befindet oder im positiven Bereich >= der Anzahl der enthaltenen Elemente. In beiden Fällen ist es Out-Of-Range und sollte daher gleich behandelt werden.

Am "aussagekräftigsten" wäre natürlich ein ScriptRuntimeError.
Dies hätte jedoch zur Folge, dass jeder Entwickler in seinen Skripten vor der Verwendung von .StrValueByIndex(), falls nicht zweifelsfrei sichergestellt werden kann, dass der Indexwert gültig ist, zwangsweise prüfen müßte, ob der Indexwert innerhalb der Grenzen liegt. Dies wäre sehr unständlich, da es keine entsprechende Methode zur Ermittlung der Anzahl an Elementen im String gibt. Oder man setzt im Skript die Exceptionbehandlung ein, um den Fehlerfall abzufangen um darauf reagieren zu können. Auch nicht das Gelbe vom Ei.

Den Ausgabetyp zu ändern und zum Beispiel "false" zurückzugeben, was natürlich im Gegensatz zu einem Leerstring eindeutig wäre, ist Aufgrund der Möglichkeit der Verkettungen auch nicht geeignet.

Alles in allem ist das "Problem" nicht optimal zu lösen.

Schauen wir uns aber einmal die Methode .webGetValueFromList() an, welche das gleiche Funktionsprinzip für eine semikolenseparierten Zeichenkette (Liste) ausführt, was der Ausführung der Methode .StrValueByIndex(";",n) entsprechen würde. Dort wird bei einem Index ausserhalb des gültigen Bereiches, egal ob positiv oder negativ, grundsätzlich ein Leerstring zurückgegeben.

Wenn ich dies alles zusammen betrachte, sollte man in Erwägung ziehen die Ausgabe von .StrValueByIndex() an die Ausgabe von .webGetValueFromList() anzugleichen und einen Leerstring zurückzugeben.

@HMMike
Copy link

HMMike commented Jan 13, 2024

Da Regascript kein try except beherrscht zum Fehlerabfangen und sich die Anzahl der Elemente nicht wie im Namensraum web. mit webGetValueListCount ermitteln lässt, neige ich dazu, bei einem Index out of bounds in beiden Fällen einen Leerstring zurückzugeben oder es bei der jetzigen Version zu belassen.

Das muss einen Kompromiss ergeben, der den Sprachumfang der Rega berücksichtigt und das Fehlen von Möglichkeiten zum exception handling.

Black

@jens-maus
Copy link
Owner

Ok, danke für all euer Feedback. Ich denke dann werde ich das in der Tat so umsetzen das für index < 0 und für index > N immer ein leerer String zurückgegeben wird. Ggf. baue ich dann noch ein, das wenn index < 0 bzw. ein invalider Wert übergeben wird (z.B. ein String) er dann im Log eine Warnung ausgibt damit man das mitbekommt das StrValueByIndex() falsch verwendet wird.

@jens-maus jens-maus removed the ❓ undecided No decision to accept or reject ticket yet label Jan 14, 2024
@jens-maus jens-maus added this to the future release milestone Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug-report Something isn't working 🏷️ ReGaHss This refs the ReGaHss component
Projects
Development

No branches or pull requests

4 participants