PBP: Warum sind Prototypen schlecht?

Begonnen von RichardCZ, 25 März 2020, 18:02:27

Vorheriges Thema - Nächstes Thema

RichardCZ

PBP führt das auf zweieinhalb Seiten 194-196 aus.

Ganz wichitg ist, dass diese Regel (verwende keine Prototypes) auch nicht alleine für sich steht.
Bevor man also soweit ist, dass man die ganzen Puzzlestücke (PBP Regeln) in ihrer Gesamtheit Überblickt, gibt es nur zwei Dinge die helfen: Geduld & Vertrauen.

Aber ich versuche mal einige FHEM-spezifischen Beispiele und Erläuterungen im Kontext loszuwerden:

CoolTux hat ja - weil er durch einige PMs mit mir ein wenig Vorsprung hat - die Protos in 73_AutoShuttersControl rausgenommen:
https://svn.fhem.de/trac/changeset?reponame=&new=21506%40trunk%2Ffhem%2FFHEM%2F73_AutoShuttersControl.pm&old=21495%40trunk%2Ffhem%2FFHEM%2F73_AutoShuttersControl.pm

Von all den Änderungen würde ich gerne eure Aufmerksamkeit auf "sub Set" richten. Was sehen wir in der vorhergehenden Version?

https://svn.fhem.de/trac/browser/trunk/fhem/FHEM/73_AutoShuttersControl.pm?rev=21495#L592

sub Set($$@) {
    my ( $hash, $name, @aa ) = @_;
    my ( $cmd, @args ) = @aa;
...


Angeblich nimmt Set ja zwei Skalare und eine Liste. Tja...
In Wirklichkeit nimmt Set drei Skalare und eine Liste, nur hat man das dritte Skalar in der Liste versteckt.
Man erzeugt also eine Illusion - nicht mehr und nicht weniger - einer formalen Definition der Übergabeparameter.

Aber das ist nur ein Nebenschauplatz. In Wirklichkeit gibt es eine wesentlich bessere Methode Parameter zu übergeben (named Arguments - siehe PBP 182/183) sobald man vielleicht mal mehr als 3 Parameter hat. Und warum sollte man einen PVC Spoiler an einen rostigen Golf pappen, wenn man einen GTR haben kann?

Also: Prototypes sind schlecht und man kann die fast immer wegschmeissen. (*)
Geduld & Vertrauen: Es werden wesentlich bessere Methoden der Parameterübergabe und Validierung vorgestellt werden, so dass niemand Prototypes nachtrauern muss.






(*) Vorsicht ist geboten, wenn irgendwo z.B. ein sub blah(\@\@) steht. Aber das scheint nicht Usus zu sein. Falls jemand dem irgendwo begegnet, frage man mich wie man das rausoperiert.

Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

justme1968

#1
ZitatAngeblich nimmt Set ja zwei Skalare und eine Liste. Tja...
nein...

es sind zwei skalare und eine liste. das die liste danach noch mal zerlegt wird ändert nichts daran.

ZitatMan erzeugt also eine Illusion - nicht mehr und nicht weniger - einer formalen Definition der Übergabeparameter.
ja. eine hilfe um auf einen blick an bestimmte information zu kommen. wenn der hingeschriebene prototyp nicht um beabsichtigten code passt ist das genau so schlecht wie ein falscher kommentar oder ein falsch benannter parameter. der prototyp verhindert bestimmte fehler. nicht alle. nicht optimal. aber manche. und er kostet nichts.

das benannte argumente einer ellenlangen liste an unbenannten parametern vorzuziehen ist ist klar, aber das hat nichts mit protypen zu tun. das eine verbessert die lesbarkeit (und fehleranfälligkeit) an der aufrufenden stelle. an der aufgerufenen stelle sieht man erst mal nichts davon.


der set und get fall ist im übrigen ein schlechtes weispiel weil im array nur die einzelnen durch leerzeichen getrennten token einer kommando eingabe stecken und man name auch noch dazu zählen könnte. der Prototyp sollte also eigentlich ($@) heissen und die verarbeitung der liste sollte man parseParams überlassen die einem dann genau den hash mit benannten parametern und einem array für den rest zurück gibt.

und wenn man im modul hash parseParams gesetzt hat passiert das sogar komplett automatisch und die Set routine wird genau damit aufgerufen.
hue, tradfri, alexa-fhem, homebridge-fhem, LightScene, readingsGroup, ...

https://github.com/sponsors/justme-1968

RichardCZ

#2
Zitat von: justme1968 am 25 März 2020, 20:58:15
der prototyp verhindert bestimmte fehler. nicht alle. nicht optimal. aber manche. und er kostet nichts.

Prototypen verursachen Fehler. Da es natürlich Du warst, der PBP für dogmatisches Teufelswerk hält, vermute ich, dass Du da nicht mehr als 2-3 Seiten gelesen hast.

Das Beispiel auf Seite 194 zeigt ziemlich deutlich welche Probleme entstehen, wenn man mit Prototypen implizites Verhalten einführt, wie z.B.

sub swap_arrays (\@\@) { ...

und dann später

swap_arrays(@sheep, @goats);

Jeder normal denkende Perl Entwickler wird sich beim Aufruf sagen, dass hier die zwei Listen zu einer geplättet werden. Protos machen es unmöglich auf das Verhalten einer Routine zu schliessen indem man sich ihren Aufruf im Programmcode anschaut. Falls man die Sache ernst nimmt, müsste man sich eigentlich IMMER die Definition ansehen.

Ich habe mir sogar die Mühe eines grep und tamtam gemacht und siehe da, es gibt doch 2-3 echt problematische Stellen im FHEM code:

FHEM/98_Modbus.pm:sub Modbus_CheckEval($\@$$);
FHEM/00_SIGNALduino.pm:sub SIGNALduino_MatchSignalPattern($\@\%\@$){
FHEM/00_SIGNALduino.pm:sub SIGNALduino_padbits(\@$) {
FHEM/95_YAAHM.pm:    $res .= "sub HouseTimeHelper(\@){\n".


Da kann man die Protos nicht einfach so entfernen. In den allermeisten anderen Fällen kann und sollte man.

Prototypen haben noch nicht einmal den Nutzen um im Fall von sub-Referenzen ... naja ... von Nutzen zu sein.

Zitatprototypes have no influence on subroutine references like \&foo or on indirect subroutine calls like &{$subref} or $subref->() .

Prototypen sind primär dazu gedacht, damit man subs definieren kann die sich wie builtins verhalten. Sagt nicht PBP, sagt die Perl Doku
https://perldoc.perl.org/perlsub.html#Prototypes

Zitatthe intent of this feature is primarily to let you define subroutines that work like built-in functions

Wenn man ganz hart ist, kann man mit Prototypen Neue Syntax in Perl einführen. Aber bitte ... in FHEM?
Diesen Anwendungsfall gibt es in FHEM einfach nicht. Ergo, werden hier die Prototypen einfach nur falsch angewendet/misbraucht.
Aber wie gesagt: Ich werde jetzt nicht das Buch rezitieren


Zitat
der set und get fall ist im übrigen ein schlechtes weispiel weil im array nur die einzelnen durch leerzeichen getrennten token einer kommando eingabe stecken und man name auch noch dazu zählen könnte. der Prototyp sollte also eigentlich ($@) heissen

Nein, der Prototyp sollte gar nichts heißen, der sollte einfach nicht da sein. Alleine schon die Tatsache, dass es für die Semantik komplett Wurst ist, ob in dem Set Fall $$@, $$$@ oder gar nix steht zeigt, dass "gar nix" die beste Alternative ist.


Zitat
und die verarbeitung der liste sollte man parseParams überlassen die einem dann genau den hash mit benannten parametern und einem array für den rest zurück gibt.

und wenn man im modul hash parseParams gesetzt hat passiert das sogar komplett automatisch und die Set routine wird genau damit aufgerufen.

Ich vermute, Du meinst

parseParams($;$$$) aus fhem.pl, nicht das fast baugleiche
parseParams($;$$$$) aus 00_MQTT

Die Verwendung von zentralen Funktionen wie parseParams anstelle irgendwelcher selbstgestrickten parsing Geschichten ist wieder ein anderes Thema und prinzipiell richtig. Sobald sich parseParams auch wäscht und die Zähne putzt, werde ich das sogar mit Nachdruck empfehlen.


Hinsichtlich Prototypes bleibt es dabei: Ist die Pest, macht es weg. Diejenigen, die neue Perl Syntax, neue Befehle etc. damit generieren wollen fühlen sich ja nicht angesprochen.
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

CoolTux

Zitat von: justme1968 am 25 März 2020, 20:58:15
der set und get fall ist im übrigen ein schlechtes weispiel weil im array nur die einzelnen durch leerzeichen getrennten token einer kommando eingabe stecken und man name auch noch dazu zählen könnte. der Prototyp sollte also eigentlich ($@) heissen und die verarbeitung der liste sollte man parseParams überlassen die einem dann genau den hash mit benannten parametern und einem array für den rest zurück gibt.

und wenn man im modul hash parseParams gesetzt hat passiert das sogar komplett automatisch und die Set routine wird genau damit aufgerufen.

Hallo Andre,

Hättest Du ein Beispielmodul für mich für genau diese automatisierte Verarbeitung. Ich würde das gerne nehmen.


Grüße
Marko
Du musst nicht wissen wie es geht! Du musst nur wissen wo es steht, wie es geht.
Support me to buy new test hardware for development: https://www.paypal.com/paypalme/MOldenburg
My FHEM Git: https://git.cooltux.net/FHEM/
Das TuxNet Wiki:
https://www.cooltux.net

justme1968

wenn du im modul hash parseParams setzt wird deine SetFn nicht mehr mit der string liste sondern mit dem ergebnis von parseParams aufgerufen.

siehe  DevelopmentModuleIntro seite im wiki
hue, tradfri, alexa-fhem, homebridge-fhem, LightScene, readingsGroup, ...

https://github.com/sponsors/justme-1968

CoolTux

Zitat von: justme1968 am 25 März 2020, 21:59:46
wenn du im modul hash parseParams setzt wird deine SetFn nicht mehr mit der string liste sondern mit dem ergebnis von parseParams aufgerufen.

siehe  DevelopmentModuleIntro seite im wiki

Das schaue ich mir mal morgen an.
Du musst nicht wissen wie es geht! Du musst nur wissen wo es steht, wie es geht.
Support me to buy new test hardware for development: https://www.paypal.com/paypalme/MOldenburg
My FHEM Git: https://git.cooltux.net/FHEM/
Das TuxNet Wiki:
https://www.cooltux.net

RichardCZ

Zitat von: RichardCZ am 25 März 2020, 21:42:32
Die Verwendung von zentralen Funktionen wie parseParams anstelle irgendwelcher selbstgestrickten parsing Geschichten ist wieder ein anderes Thema und prinzipiell richtig. Sobald sich parseParams auch wäscht und die Zähne putzt, werde ich das sogar mit Nachdruck empfehlen.

https://gl.petatech.eu/root/HomeBot/-/commit/7aa4faaaf84ec090ef5f9f3b10d6876d39e89f22

ist aber noch nicht ganz fertig (und vor allem ungestestet). Für heute mach ich lieber Feierabend.
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

justme1968

zurück zum thema prototypen:

die beiden beispiele die im buch verwendet werden um prototypen als generell schlecht darzustellen zeigen nur eins: das sich die semantik ändern wenn man protypen willenlos einführt. das heisst im übrigen auch das sich diese beim grundlosen entfernen der prototypen ebenfalls ändern. das ist eine zusätzliche potentielle fehlerquelle.

sie zeigen nicht das das die verwendung von protypen um den code intent klarer zu machen schlecht oder falsch ist. sie sagen nichts darüber aus wie es mit code ausschaut der direkt mit verwendung von protypen entstanden ist. das in beiden beispielen listen vorkommen zeigt nur das es mit protypen und listen probleme geben kann wenn man nicht weiss was man tut. sie sagen nicht das es nicht auch andere fälle gibt. das die beispiele die verwendet werden extra so gestrickt sind das der code 'zufällig' schon mehr kann und im table lookup fall auch funktioniert ist ein schlechter taschenspieler trick. wenn die implementierung damit nicht klar kommen würde wäre das ergebnis des falschen aufrufs schlicht und einfach ein fehler.


prototypen sind teil des designs. sie führen nicht implizites verhalten ein sondern machen das verhalten explizit. sie erlauben auf einen ersten kurzen blick einen einblick in die idee hinter einer routine. sie ermöglichen eine gewisse art von fehler frühzeitig zu erkennen. nicht immer. aber eben auch nicht niemals.

ein min($$) macht ganz klar das diese routine für genau zwei parameter bestimmt ist. ein min(@) hingegen ist für beliebig viele paramter. ein falscher aufruf (3 statt 2 parameter) von min kann frühzeitig erkannt werden. eine version ohne protypen gibt ohne genaues anschauen des quelltextes keinerlei information darüber ob ein aufruf mit drei parametern gültig ist. wenn man sich bei der implementierung nichts weiter gedacht hat liefert er einfach falsche ergebnisse ohne fehlzuschlagen.

bei eine änderung der routine von nur zwei auf mehr parameter mit anpassen des protypen zeigt das der aufruf kompatibel geblieben ist. bei einem potentiellen umbau von beliebig vielen (wer braucht das schon) auf nur drei parameter würde der falsche aufruf erkannt.

das nur bestimmte probleme damit erkannt werden ist unbenommen. das heisst aber nicht das man deshalb auf alle möglichkeiten verzichten sollte.

es kommt sie immer darauf an das man weiss was man tut.

es gibt in fhem ganz sicher viele dinge die man anders oder sogar besser machen kann. es gibt echte fehler die man beheben kann. es gibt von architektur über dokumentation und optimierung bis hin zu code reviews möglichkeiten sich einzubringen und beizutragen. ganz zu schweigen vom schreiben neuer module. das entfernen der protypen gehört ganz sicher nicht dazu. 

ps: der ganze sinn und zweck von parseParams ist das parsen einer kommandozeile in einen ergebnis hash und array. die beiden variablen h und a zu nennen ist bei weitem nicht das schlechteste. x und y wäre schlechter. hash und array oder result_hash und result_array wären länger aber nicht besser.

pps: wenn ein grosser teil der änderungen an einer funktionierenden routine daraus besteht kommentarlos einen anderen stil zu verwenden (aufrufe mit/ohne klammern, ?: statt if, ...) ist das oft keine frage von besser sondern nur von anders.

ppps: die umstellung mit verwendung des defined-or finde ich schön. lustigerweise war dessen erste verwendung in fhem damals auch das erste problem das jemand anders mit einem meiner module hatte. den gibt es nämlich erst ab perl 5.10 und zumindest damals waren noch viele ältere perl versionen in verwendung.

hue, tradfri, alexa-fhem, homebridge-fhem, LightScene, readingsGroup, ...

https://github.com/sponsors/justme-1968

justme1968

in deiner umgebauten parseParams ist in zeile 5839 noch ein ; zu viel.
hue, tradfri, alexa-fhem, homebridge-fhem, LightScene, readingsGroup, ...

https://github.com/sponsors/justme-1968

justme1968

das verhalten ist auch noch nicht mit der alten version identisch:

parsen von 'set name test1 test2=abc test3 "test4 test4" test5="test5 test5" test6=\'test6=test6\' test7= test8="\'" test9=\'"\' {my $x = "abc"} test10={ { my $abc ="xyz" } }'

sollte das hier geben:$VAR1 = [
          'set',
          'name',
          'test1',
          'test3',
          'test4 test4',
          '{my $x = "abc"}'
        ];
$VAR2 = {
          'test9' => '"',
          'test5' => 'test5 test5',
          'test6' => 'test6=test6',
          'test2' => 'abc',
          'test8' => '\'',
          'test7' => '',
          'test10' => '{ { my $abc ="xyz" } }'
        };

aktuell gibt es aber das hier: $VAR1 = [
          'set',
          'name',
          'test1',
          'test3',
          'test4 test4',
          '{my $x = "abc"} test10={ { my $abc ="xyz" } }'
        ];
$VAR2 = {
          'test6' => 'test6=test6',
          'test5' => 'test5 test5',
          'test7' => '',
          'test2' => 'abc',
          'test9' => '"',
          'test8' => '\''
        };


d.h. die unterschieldliche behandlung von test10

ohne genau geschaut zu haben vermute ich es liegt am geänderten zusammenfassen und zählen der geklammerten teile. du zählst alles auf einen schlag, korrekt ist aber nur bis zur gleichen klammer ebene herunter zu zählen.
hue, tradfri, alexa-fhem, homebridge-fhem, LightScene, readingsGroup, ...

https://github.com/sponsors/justme-1968

RichardCZ

Zitat von: justme1968 am 26 März 2020, 08:35:29
prototypen sind teil des designs. sie führen nicht implizites verhalten ein sondern machen das verhalten explizit. sie erlauben auf einen ersten kurzen blick einen einblick in die idee hinter einer routine.

Wie bereits erwähnt, steht die Empfehlung Prototypen zu knicken nicht alleine für sich da, sie ist Teil eines umfassenden Regelwerks (Habitual concepts) und fügt sich in dieses ein.

Das Argument, Prototypen erlauben "durch einen einfachen Blick darauf" eine wie auch immer geartete Analyse bzw. Erkenntnis des Codes dahinter halte ich für einen billigen Taschenspielertrick.

Für gewöhnlich macht so etwas die Dokumentation. Nur weil man diese nicht hat, ebenso wie man tests nicht hat, ebenso wie man keine klare Struktur bei parametervalidierungen hat bedeutet das noch lange nicht, dass man durch die missbräuchliche Art und Weise - also wozu man glaubt Prototypen zu verwenden - all diese Defizite auf magische Art und Weise behebt.

Du klammerst Dich hier an eine Fata Morgana.

Zitat
das nur bestimmte probleme damit erkannt werden ist unbenommen. das heisst aber nicht das man deshalb auf alle möglichkeiten verzichten sollte.

Es gibt bessere Möglichkeiten. Nochmal: Warum soll ich einen gammligen PVS Spoiler auf einen rostigen Golf pappen wenn ich einen GTR haben kann?

Zitat
es kommt sie immer darauf an das man weiss was man tut.
...
das entfernen der protypen gehört ganz sicher nicht dazu. 

Das sowieso (wissen was man tut). Und das Entfernen sinnloser Cargo Cult Konzepte gehört in erster Linie dazu.

Zitat
ps: der ganze sinn und zweck von parseParams ist das parsen einer kommandozeile in einen ergebnis hash und array. die beiden variablen h und a zu nennen ist bei weitem nicht das schlechteste. x und y wäre schlechter. hash und array oder result_hash und result_array wären länger aber nicht besser.

Eine Variable @a zu nennen ist extrem schlecht. Wer das tut, hat weder die Empfehlung verstanden, man solle im gleichen Scope nicht gleiche Variablennamen verwenden für verschiedene Typen (also z.B. nicht @param, %param und $param im gleichen Scope)

Wenn man das weiß und auch weiß, dass $a und $b spezielle globale Variablen sind, macht man diesen Unsinn nicht. Schon gar nicht verteidigt man ihn.

Zitat
pps: wenn ein grosser teil der änderungen an einer funktionierenden routine daraus besteht kommentarlos einen anderen stil zu verwenden (aufrufe mit/ohne klammern, ?: statt if, ...) ist das oft keine frage von besser sondern nur von anders.

Da müsste man natürlich auch informiert sein und wissen warum bei BUILTINS (nicht Funktionen) die Argumente bevorzugt nicht mit Klammern angegeben werden.

Man kann sich natürlich spreizen und erstmal "dagegen sein". Kein Problem. Man könnte sich auch ein wenig kooperativ zeigen und von Synergien profitieren. So würde es mir z.B. helfen, wenn Du eine gute represäntative Liste der typischen Argumente an parseParams liefern würdest. Dann könnte ich eine Testsuite und einen Benchmark schreiben.

Ich hielte sowas für sinvoller als Kreuzzüge.
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

justme1968

ZitatSo würde es mir z.B. helfen, wenn Du eine gute represäntative Liste der typischen Argumente an parseParams liefern würdest. Dann könnte ich eine Testsuite und einen Benchmark schreiben.

s.o. :)
hue, tradfri, alexa-fhem, homebridge-fhem, LightScene, readingsGroup, ...

https://github.com/sponsors/justme-1968

RichardCZ

Zitat von: justme1968 am 26 März 2020, 08:56:17
das verhalten ist auch noch nicht mit der alten version identisch:

Ich schrieb ja gestern um 23Uhr-irgendwas es sei noc hnicht fertig und nicht getestet.
Habe dann abends aber noch einen fix geliefert.

https://gl.petatech.eu/root/HomeBot/-/commit/335a787de26daf172779ae370c4093694ff4cf33

$ tr_count.pl
$VAR1 = [
          'set',
          'name',
          'test1',
          'test3',
          'test4 test4',
          '{my $x = "abc"}'
        ];
$VAR2 = {
          'test9' => '"',
          'test8' => '\'',
          'test5' => 'test5 test5',
          'test10' => '{ { my $abc ="xyz" } }',
          'test2' => 'abc',
          'test7' => '',
          'test6' => 'test6=test6'
        };

Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

justme1968

ZitatDa müsste man natürlich auch informiert sein und wissen warum bei BUILTINS (nicht Funktionen) die Argumente bevorzugt nicht mit Klammern angegeben werden.

deshalb steht da oben ja auch: kommentarlos...

ich sehe nirgendwo eine information oder erklährung
hue, tradfri, alexa-fhem, homebridge-fhem, LightScene, readingsGroup, ...

https://github.com/sponsors/justme-1968

RichardCZ

Zitat von: justme1968 am 26 März 2020, 09:02:04
s.o. :)

Bischen spät aber besser als nix. Mehr davon (Grenzfälle etc.). Hinsichtlich dieses Testfalls verhalten sich die Subs nun gleich

sub pP_mud_puddle {
    parseParams($test);
}

sub pP_pixie_dust {
    parseParams2($test);
}

timethese(100000, {
                  'pP1'  => \&pP_mud_puddle,
                  'pP2'  => \&pP_pixie_dust,
                 });


Benchmark: timing 100000 iterations of pP1, pP2...
       pP1:  6 wallclock secs ( 5.70 usr +  0.00 sys =  5.70 CPU) @ 17543.86/s (n=100000)
       pP2:  5 wallclock secs ( 5.16 usr +  0.00 sys =  5.16 CPU) @ 19379.84/s (n=100000)


Die neue ist so 10% schneller - und kürzer und übersichtlicher und immer noch nicht fertig. Noch eine Mannwoche Arbeit und man kann die vielleicht zur Verwendung empfehlen.  ;)
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.