Peer Review von Richards Gewurschtel

[RichardCZ]

https://gl.petatech.eu/root/HomeBot/-/jobs/103

1395 tests. Ich frohlocke. Im übrigen: hinter der unscheinbaren Zeile 32

t/HoBo/01_func.t .............................. ok

verbergen sich 40 tests

ok 1 - goodDeviceName: without params
ok 2 - goodDeviceName: undef param
ok 3 - goodDeviceName: invalid name
ok 4 - goodDeviceName: invalid name 2
ok 5 - goodDeviceName: valid name
ok 6 - goodDeviceName: numeric context is ok
ok 7 - goodDeviceName: we do not care about superfluous parameters
ok 8 - goodDeviceName: invalid "hidden" name
ok 9 - goodDeviceName: valid but questionable name
ok 10 - goodReadingName: without params
ok 11 - goodReadingName: undef param
ok 12 - goodReadingName: invalid name
ok 13 - goodReadingName: valid name
ok 14 - goodReadingName: numeric context is ok
ok 15 - goodReadingName: we do not care about superfluous parameters
ok 16 - goodReadingName: valid hidden name
ok 17 - goodReadingName: invalid hidden name
ok 18 - goodReadingName: valid, but questionable name
ok 19 - goodReadingName: valid name with all character classes
ok 20 - makeDeviceName: without params
ok 21 - makeDeviceName: valid name
ok 22 - makeDeviceName: invalid name - corrected
ok 23 - makeReadingName: without params
ok 24 - makeReadingName: nothing to convert
ok 25 - makeReadingName: convert invalid char
ok 26 - makeReadingName: hidden can do anything
ok 27 - ReadingsTimestamp: without params
ok 28 - ReadingsTimestamp: explicit undefs
ok 29 - ReadingsTimestamp: default shortcut
ok 30 - ReadingsTimestamp: default because device & reading not present
ok 31 - ReadingsTimestamp: default because reading not present
ok 32 - ReadingsTimestamp: got reading
ok 33 - ReadingsAge: without params
ok 34 - ReadingsNum: without params
ok 35 - ReadingsVal: without params
ok 36 - OldReadingsAge: without params
ok 37 - OldReadingsNum: without params
ok 38 - OldReadingsTimestamp: without params
ok 39 - OldReadingsVal: without params
ok 40 - _oldreadings: without params
1..40

Und ich würde "den Verantwortlichen" doch mal stark ans Herz legen sowohl die Unit-Tests wie auch die Codeänderungen zu beobachten. Da stimmt nämlich einiges vorne und hinten nicht in FHEM - sowohl was Code, als auch was WIki betrifft.
Genauer ausgeführt habe ich das unter: https://gl.petatech.eu/root/HomeBot/-/wikis/FHEM_CodeDocs
Aber Vorsicht! Schnappatmungsgefahr...

[rudolfkoenig]

Harte Fakten:...Nein, goodReadingName kann nicht direkt im if verwendet werden

Ist mir neu. Muss ich was aendern?

fhem> setreading global ätschbätsch value
global: bad reading name 'ätschbätsch' (allowed chars: A-Za-z/\d_\.-)

Kriegen wir eigentlich eine Richtigstellung auf deine Seite oder muessen wir Falschaussagen einfach so hinnehmen?

[RichardCZ]

Ist mir neu. Muss ich was aendern?

Ja, die Dioptrien bei der Brille. 

fhem> setreading global ätschbätsch value
global: bad reading name 'ätschbätsch' (allowed chars: A-Za-z/\d_\.-)

Nochmal lesen. Da ist ein Punkt davor.

Kriegen wir eigentlich eine Richtigstellung auf deine Seite oder muessen wir Falschaussagen einfach so hinnehmen?

Ich weiß nicht. Was sagt Dein Optiker?

[CoolTux]

Ja, die Dioptrien bei der Brille. 

Nochmal lesen. Da ist ein Punkt davor.

Ich weiß nicht. Was sagt Dein Optiker?

Verstehe ich nicht. Wo ist da ein Punkt davor. Kann mich mal bitte einer aufklären.
Und was hat das mit der vorher erwähnten If Condition zu tun.

Code Auswählen Erweitern

hem> setreading global ätschbätsch value
global: bad reading name 'ätschbätsch' (allowed chars: A-Za-z/\d_\.-)

Ist doch korrekt. Umlaute sind nicht erlaubt. Oder über sehen ich da auch was?

[RichardCZ]

Leute... ihr geht jetzt alle geschlossen zum Optiker.

.ätschbätsch

Kann ich hier größere Schrift machen?

[rudolfkoenig]

Der Optiker sagt, dass ich den Punkt uebersehen habe, Punkt fuer Dich.
Die Pruefung in diesem Fall zu lassen ist Absicht, weil Modulautoren auf eine Ausnahme beharrt haben.

"Nein, goodReadingName kann nicht direkt im if verwendet werden" ist weiterhin eine Falschaussage.

[RichardCZ]

Der Optiker sagt, dass ich den Punkt uebersehen habe, Punkt fuer Dich.
Die Pruefung in diesem Fall zu lassen ist Absicht, weil Modulautoren auf eine Ausnahme beharrt haben.

Ist ja ok, könnte man auch in der Doku sagen. Schliesslich ist diese ja für Modulautoren.
Oder gibt es noch anderes Geheimwissen was nur Super-Modulautoren vorbehalten ist?

"Nein, goodReadingName kann nicht direkt im if verwendet werden" ist weiterhin eine Falschaussage.

Ach tatsächlich?

$ grn.pl
Use of uninitialized value in string eq at ./grn.pl line 16.


grn.pl:

Code Auswählen Erweitern

#!/usr/bin/env perl

use strict;
use warnings;


sub
goodReadingName($)
{
  my ($name) = @_;
  return undef if(!$name);
  return ($name =~ m/^[a-z0-9._\-\/]+$/i || $name =~ m/^\./);
}


if (goodReadingName(0) eq '0') {
    print "Tadaaa\n";
}

Und das trotz Prototypen! Nit möööchlich!

Noch ein Punkt für mich?

edit:

Oder ich gehe ganz nach Vorschrift vor. Doku sagt Readinname kann Ziffer, Zeichen, punkt, Underscore sein.

Code Auswählen Erweitern

my $readingname = '0'; # laut Doku valide

if (goodReadingName($readingname) == 1) { # laut Doku liefert das 0 oder 1 zurück
    print "Tadaaa\n";
}


Ich habe alles richtig gemacht, und dennoch

$ grn.pl
Use of uninitialized value in numeric eq (==) at ./grn.pl line 17.

Die sind alle so böse zu mir. denkt sich da der Modulautor.

[rudolfkoenig]

Nein, es bleibt eine Falschaussage: die Funktion kann man im if verwenden.

Die Doku im Wiki will ich nicht verteidigen, die ist zwar richtig gemeint, man darf nur nicht jede Buchstabe auf die Waage liegen.
In der Wiki sind vermutlich noch etliche Fehler: ich habe die Artikel weder verfasst, noch habe ich es vor sie zu korrigieren.

Warum man das Beduerfnis hat, aus diesen Ungenauigkeiten so ein Aufschrei und noch dazu in so einem herablassenden Ton zu machen, ist mir ein Raetsel.

[RichardCZ]

Nein, es bleibt eine Falschaussage: die Funktion kann man im if verwenden.

Das ist der Unterschied zwischen uns. Ich bin präzise - Du nicht.

Hast mal eben das Wörtchen "direkt" unterschlagen. Klar kann man die die Funktion im if verwenden

if (defined goodReadingname($blah) && goodReadingname($blah) == 0) {

aber eben nicht direkt - also ohne den defined-Schutz. ABer warum sollte man das tun, wenn man doch goodReadingname einfach verbessern kann?

Die Doku im Wiki will ich nicht verteidigen, die ist zwar richtig gemeint, man darf nur nicht jede Buchstabe auf die Waage liegen.
In der Wiki sind vermutlich noch etliche Fehler: ich habe die Artikel weder verfasst, noch habe ich es vor sie zu korrigieren.

Klar sind dort noch viele Fehler. Das hindert aber zappendustre Zeloten wie zap nicht daran z.B. "return undef;" im Code zu behalten
"weil es die Doku so sagt".

Könntest ihm ja mal sagen, dass die Doku nicht heilig ist und man da nicht jeden Buchstaben auf die Waage legen soll... 

Warum man das Beduerfnis hat, aus diesen Ungenauigkeiten so ein Aufschrei und noch dazu in so einem herablassenden Ton zu machen, ist mir ein Raetsel.

Das Problem ist nicht nur im Wiki, das Problem ist ja auch im Code. Und wieder sind wir angekommen bei "Najaaaa vielleicht ne kleine Ungenauigkeit, aber der Code ist ja eigentlich in Ordnung." Da bleibt einem nicht viel anderes übrig als sich darüber lustig zu machen. Das kann man als herablassend interpretieren - ja.

Also ich glaube ja, einige hier hätten im 14. Jh sehr gute Kirchenvertreter abgegeben und ich wäre längst Asche. 
Bin ich erleichtert, dass wir 2020 A.D. schreiben.

Aber weisste was das richtig Traurige an der ganzen Sache ist?

Du hast im HoBo Repository gefixten Code. Sogar formal getestet. Brauchst Dich nur bedienen. Aber nein - da werde ich lieber der Falschaussage, der herablassenden Art etc. bezichtigt. Du willst herablassend? Ok. Dieses Verhalten nenne ich "stur", hart an der Grenze zu "dumm".
Denk' drüber nach - wenn möglich.

[rudolfkoenig]

aber eben nicht direkt - also ohne den defined-Schutz.

Klar doch: if(goodReadingname($bla)). Steht im Bibel nicht, dass == 0 eine Todsuende ist?

Du hast im HoBo Repository gefixten Code. Sogar formal getestet.

Das ist deine Sicht.
Meine Sicht: formal getestet ist sinnlos, solange man nicht so recht weiss, wozu der Code da ist. Von mir nicht mehr wartbar, da komplett umgebaut, und die Aenderungshistorie verlorengegangen ist. Ungetestet in Real-Life.

[RichardCZ]

Klar doch: if(goodReadingname($bla)). Steht im Bibel nicht, dass == 0 eine Todsuende ist?

== 0 wüsste ich nicht. / 0 vielleicht.

if(goodReadingname($bla))

funktioniert nur durch Zufall, weil Perl "undef" zu "false" evaluiert (was das FHEM Wiki als "0" beschreibt)
Wenn es nicht so irre wäre, wäre es ja richtig lustig.

Wehe ... wehe! irgendjemand legt die Wiki Worte auf die Waagschale und erwartet da nicht undef zurück.

Das ist deine Sicht.
Meine Sicht: formal getestet ist sinnlos, solange man nicht so recht weiss, wozu der Code da ist. Von mir nicht mehr wartbar, da komplett umgebaut, und die Aenderungshistorie verlorengegangen ist. Ungetestet in Real-Life.

Ich verstehe die konservative Herangehenseweise. Aber würde ich nicht in meinem Elfenbeinturm über den Dingen schweben, würde ich Dich jetzt der Falschaussage bezichtigen:

a) die Änderungshistorie ist natürlich da (gibt im GitLab zu jeder Datei den "History" Button).
b) es ist tatsächlich mehr getestet als in Real-Life, weil sogar die Grenzfälle, die in Real-Life nie getestet wurden (weil man sonst die Fehler gemerkt hätte) getestet sind.
c) wenn man solchem Code nie die Chance auf "Real-Life" gibt, dann wird das "Ungetestet in Real-Life" zu einer selbterfüllenden Prophezeiung.

ABER:

Selbst dieser "total umgebaute Code" ist m.E. noch nicht fertig. Das wäre er, wenn es uns statt dieser Rhetorikübungen gelingen würde, ein wenig Synergie zu finden. Z.B. wo Du - mit all Deinem Hintergrundwissen zu FHEM in "Real-Life" - und ich - mit meinem Perl Wissen - den Code finalisieren könnten.

Offene Fragen sind u.a. nämlich:

//////
......
-------.....////.....

will man das tatsächlich als valide Readingnames haben? Wen nein, was will man genau haben? Das wäre IMHO produktiv, weil bis man das festgenagelt hat bleibt die ganze Geschichte in einer diffusen Wolke. Und außerdem möchte ich natürlich angeben wie tolle reguläre Ausdrücke ich schreiben kann.

Pah! Ihr habt mich noch nicht negative Lookbehinds machen sehen!

[rudolfkoenig]

Offene Fragen sind u.a. nämlich:...

Ich weiss nicht, wo ich die Grenze ziehen soll: ist A--B noch ok, oder -AB ?
Readingnamen werden in etlichen Modulen aus Input generiert.
Was ist wenn jemand nicht druckbare Zeichen nach - oder . konvertiert?

Ich weiss nicht, warum ich mich damit beschaeftigen soll, Konstellationen, die zwar kaputt ausschauen, aber nicht stoeren, zu beheben.
Ich bin daran gescheitert die Laenge auf 64 Zeichen zu beschraenken, und DAS stoert mich.

a) die Änderungshistorie ist natürlich da (gibt im GitLab zu jeder Datei den "History" Button).

Klar, macht aber die Verfolgung kompliziert, weil ich erst den Code vor dem Umbau auschecken muss, um danach mit blame zu sehen, wegen welchem Beitrag eine Zeile eingebaut wurde. Vorher irgendwie rausfinden, welcher neuen Zeile welche Alte entspricht, und ob das Problem durch die Aenderung gekommen ist, oder vorher schon da war. Ist nicht viel Unterschied mehr zu "Historie ist weg"

b) es ist tatsächlich mehr getestet als in Real-Life, weil sogar die Grenzfälle, die in Real-Life nie getestet wurden (weil man sonst die Fehler gemerkt hätte) getestet sind.

Das ist so nicht richtig: ob durch den Umbau Real-Life Probleme reingekommen sind oder nicht, weiss man nicht, insofern ist der "Real-Life" getestet Siegel erstmal weg. Es wird nur getestet, was der Tester fuer moeglich haelt, und nach meiner Erfahrung sind Tester (und Programmierer) nicht so erfinderisch, wie Real-Life.

[RichardCZ]

Ich weiss nicht, wo ich die Grenze ziehen soll: ist A--B noch ok, oder -AB ?
Readingnamen werden in etlichen Modulen aus Input generiert.
Was ist wenn jemand nicht druckbare Zeichen nach - oder . konvertiert?

Ich weiss nicht, warum ich mich damit beschaeftigen soll, Konstellationen, die zwar kaputt ausschauen, aber nicht stoeren, zu beheben.
Ich bin daran gescheitert die Laenge auf 64 Zeichen zu beschraenken, und DAS stoert mich.

Das stört mich auch, denn momentan ist es echt unbegrenzt. Wenn ich jetzt den Roman "KriegUndFrieden" in einen Readingname verwandle und in der Config verewige, dann passieren bestimmt auch lustige Sachen.

Also gut. 64 Zeichen nö. Aber 128? 256? 256000? Es muss doch möglich sein einen Limes Superior anzugeben. So lange man das nicht tut ... siehe mein Avatar-Bild.

Und noch hat niemand von "beheben" gesprochen.  "Definieren" würde mir ja in erster Annäherung reichen.
Das "wir alle" momentan nicht wissen wo man die Grenze ziehen soll, würde ich eher als Bestätigung sehen ebendiese Grenze zu finden und nicht "Klappe zu , Deckel drauf wird schon irgendwie laufen".

Und dann kann man noch über andere Sachen nachdenken - das mache ich dann in der HoBo Freiheit - wie zum Beispiel bidirektionale Konversion.
Warum muss unbedingt ä -> _ Information verlorengehen? Insbesondere da es ja RFC3492 gibt bzw. https://de.wikipedia.org/wiki/Punycode

Das ist so nicht richtig: ob durch den Umbau Real-Life Probleme reingekommen sind oder nicht, weiss man nicht, insofern ist der "Real-Life" getestet Siegel erstmal weg. Es wird nur getestet, was der Tester fuer moeglich haelt, und nach meiner Erfahrung sind Tester (und Programmierer) nicht so erfinderisch, wie Real-Life.

Nach meiner Erfahrung kommt das sehr auf die Tester und Programmierer an.

[rudolfkoenig]

Also gut. 64 Zeichen nö. Aber 128? 256? 256000? Es muss doch möglich sein einen Limes Superior anzugeben. So lange man das nicht tut ... siehe mein Avatar-Bild.

Fuehl dich frei das mit dem Neinsager auszudiskutieren.
Es geht darum, als "Meta-Modul" an beliebige ReadingNamen von anderen Modulen ein Praefix (oder war das Suffix?) ranhaengen zu duerfen.
Nach dem Motto: immer zweimal mehr wie du.

[RichardCZ]

Fuehl dich frei das mit dem Neinsager auszudiskutieren.
Es geht darum, als "Meta-Modul" an beliebige ReadingNamen von anderen Modulen ein Praefix (oder war das Suffix?) ranhaengen zu duerfen.
Nach dem Motto: immer zweimal mehr wie du.

Ok. Ich beschränke das bei mir jetzt auf 256 und bevor ich nichts Gegenteiliges höre, gilt das auch für Devicenamen.

/////////////

immer noch guter Readingname?



edit:

https://gl.petatech.eu/root/HomeBot/-/commit/11e2b848ddfa58385198939275551242b6dda718

Da fackelt unser politisch und gendermäßig korrektes internationales multi-kulti HoBo-Entwicklerteam nicht lang rum.



edit2 - a.k.a. "einen hab' ich noch":

Mich hat die Testsuite tatsächlich jetzt schon vor einem Bug bewahrt, denn ich hatte bei goodDeviceName tatsächlich geschrieben

    return $name =~ m{\A                     # return truth value (true/false) of this rx match
                      [a-z0-9_]              # start with character, digit or underscore
                      [a-z0-9._]{1,255}      # continue, but now can also contain '.', max 255 chars
                      \z}xmsi;

*Boom*

not ok 6 - goodDeviceName: numeric context is ok
#   Failed test 'goodDeviceName: numeric context is ok'
#   at lib/HoBo/Test.pm line 32.
#          got: ''
#     expected: '1'
not ok 7 - goodDeviceName: we do not care about superfluous parameters
#   Failed test 'goodDeviceName: we do not care about superfluous parameters'
#   at lib/HoBo/Test.pm line 32.
#          got: ''
#     expected: '1'

Klar, wer ist auch so blöd und ersetzt * durch {1,255}

    return $name =~ m{\A                     # return truth value (true/false) of this rx match
                      [a-z0-9_]              # start with character, digit or underscore
                      [a-z0-9._]{0,255}      # continue, but now can also contain '.', max 255 chars
                      \z}xmsi;

Muss es natürlich im "Real-Life" heissen.