Autor Thema: PBP: return undef;  (Gelesen 1166 mal)

Offline RichardCZ

  • Tester
  • Sr. Member
  • ****
  • Beiträge: 597
  • WHIP! HoBo war gestern.
    • Experimenteller FHEM Fork
PBP: return undef;
« am: 27 März 2020, 22:24:45 »
Es ist ja schon an einigen Stellen durchgedrungen: return undef; ist doof. Perlcritic hält das für ein "Severity 5" Problem, also in der schlimmsten Liga. Warum eigentlich? Wo ich schon dabei bin, handle ich gleich zwei Sachen ab: "explizites Return" und "return OHNE undef"

PBP sagt man soll IMMER ein return verwenden, also keine Impliziten Returns. Was ist ein Impliziter Return? Das hier:

Code: (perl) [Auswählen]
sub implicit {
    my $foo = shift;

    if (! defined $foo) {
        print "Tadaaa";
    }
    else {
        another_sub($foo);
    }
}

Hier weiß man als Nicht-Perl-Crack eigentlich nicht auf den ersten Blick ob bzw. welche Rückgabewerte die Funktion "implicit" zurückliefert. Und das ist falsch: Der Code sollte immer so geschrieben sein, dass man sowas möglichst auf den ersten Blick sieht. PBP 197-199 sagt also nichts anderes, als dass man da ein explizites return reinpflanzen soll, damit es nix zu diskutieren oder vermuten gibt.

Will ich einen bestimmten Wert zurückhaben oder ist mir das wurst? Selbst wenn es mir wurst ist, dann eben explizit:

Code: (perl) [Auswählen]
sub explicit {
    my $foo = shift;

    if (! defined $foo) {
        print "Tadaaa";
    }
    else {
        another_sub($foo);
    }

    return;
}

Ansonsten liefere ich eben Werte zurück die der Aufrufende auswertet - mit einem expliziten return $X.



Hat man sich das im PBP oben durchgelesen, dann stösst man gleich im Anschluss auf die Weisung, man möchte bitte nie return mit undef verwenden.

Wer es also bislang ganz besonders aufrichtig gemeint hat und ein "return undef;" verwendete (5000+ Vorkommen in der Statistik machen das zum zweithäufigsten "PBP Verstoß") hat der Sache einen Bärendienst erwiesen:

Code: (perl) [Auswählen]
sub true_failure {
    return undef;
}

my @back = true_failure();

if (@back) {
    print "WTF??? TRUE???\n";
}

Sollte man das Pech haben und die eigene Subroutine mit ihrem tapferen "return undef;" wird im Listkontext (wantarray - also der Aufrufende erwartet eine Liste) aufgerufen, dann liefert man (undef) zurück. Nicht etwa eine undefinierte/leere Liste. Man liefert eine wohldefinierte Liste die eben ein Element - nämlich undef - enthält, zurück. Boom.

Hätte hätte Fahrradkette da nur "return;" gestanden, dann wäre sowohl im Skalaren, wie auch im Listkontext die Sache so abgelaufen, wie man sich das wohl vorgestellt hat.

Daher: return undef; -> return;

Eigentlich sollte man in der Lage sein das hart im Editor ersetzen zu lassen und es sollte auch folgenlos bleiben.
Falls ihr irgendwo einen Code habt, der mit "return undef;" funktioniert, mit "return;" aber nicht,...
... dann ist das ein Verbrechen gegen die Menschlichkeit und zuständig ist https://de.wikipedia.org/wiki/Internationaler_Strafgerichtshof
 :o
« Letzte Änderung: 27 März 2020, 23:24:17 von RichardCZ »
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

Online CoolTux

  • Moderator
  • Hero Member
  • ***
  • Beiträge: 27283
Antw:PBP: return undef;
« Antwort #1 am: 27 März 2020, 22:32:19 »
Wie Lustig, genau da bin ich gerade dabei.
Doch wie schaut es aus wenn ein return an eine Bedingung geknüpft ist. perlcritic -4 meckert das an

return $self->{ $self->{shuttersDev} }{AttrUpdateChanges}{$attr}
      if (  defined( $self->{ $self->{shuttersDev} }{AttrUpdateChanges} )
        and
        defined( $self->{ $self->{shuttersDev} }{AttrUpdateChanges}{$attr} ) );
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/
Mein Dokuwiki:
https://www.cooltux.net

Offline RichardCZ

  • Tester
  • Sr. Member
  • ****
  • Beiträge: 597
  • WHIP! HoBo war gestern.
    • Experimenteller FHEM Fork
Antw:PBP: return undef;
« Antwort #2 am: 27 März 2020, 22:39:58 »
return $self->{ $self->{shuttersDev} }{AttrUpdateChanges}{$attr}
      if (  defined( $self->{ $self->{shuttersDev} }{AttrUpdateChanges} )
        and
        defined( $self->{ $self->{shuttersDev} }{AttrUpdateChanges}{$attr} ) );

Bestimmt bekommst Du so eine Meldung:

Subroutine xxx does not end with "return" at line yy, column z.  See page 197 of PBP.  (Severity: 4)

Das ist die erste Baustelle die ich beschrieben habe, und sehr verwandt mit dem

my $x = 1 if ($tralala);

was wir anderweitig besprochen haben. Im obigen Fall ist ja die IF Bedingung nicht immer true, also wird auch nicht immer das Return ausgeführt. Das merkt Perlcritic - und Du solltest Dir überlegen, was explizit (mit einem zusätzlichen return) im komplementären Fall zurückgegeben werden sollte.
« Letzte Änderung: 27 März 2020, 22:41:44 von RichardCZ »
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.
Informativ Informativ x 1 Liste anzeigen

Offline Wzut

  • Developer
  • Hero Member
  • ****
  • Beiträge: 4544
Antw:PBP: return undef;
« Antwort #3 am: 28 März 2020, 08:18:18 »
Ich (und vermutlich x andere) haben irgendwann in ihren Anfängen versucht sich an Richtlinen zu halten , u.A.
https://wiki.fhem.de/wiki/DevelopmentModuleIntro#X_Set
Zitat
Als Rückgabewert $error kann eine Fehlermeldung in Form Zeichenkette zurückgegeben werden. Der Rückgabewert undef bedeutet hierbei, dass der Set-Befehl erfolgreich durchgeführt wurde
Ich für meinen Teil habe das halt nicht nur auf _Set bezogen sondern dann gnadenlos immer benutzt wenn es nichts zurück zu geben gab.
Zitat
Eigentlich sollte man in der Lage sein das hart im Editor ersetzen zu lassen und es sollte auch folgenlos bleiben
Klar kann der Editor das machen, ob es wirklich folgenlos bleibt wird ein Test des Moduls zeigen.
Bei eigenen subs bin ich mir da relativ sicher, bis auf den Fall wo ich das undef selbst erwartet habe, mal schauen wo genau das war.   
Maintainer der Module: MAX, MPD, UbiquitiMP, UbiquitiOut, SIP, BEOK, readingsWatcher

Offline RichardCZ

  • Tester
  • Sr. Member
  • ****
  • Beiträge: 597
  • WHIP! HoBo war gestern.
    • Experimenteller FHEM Fork
Antw:PBP: return undef;
« Antwort #4 am: 28 März 2020, 08:32:23 »
Klar kann der Editor das machen, ob es wirklich folgenlos bleibt wird ein Test des Moduls zeigen.
Bei eigenen subs bin ich mir da relativ sicher, bis auf den Fall wo ich das undef selbst erwartet habe, mal schauen wo genau das war.

Code: (perl) [Auswählen]
sub X_Undef($$)   
{                     
my ( $hash, $name) = @_;       
DevIo_CloseDev($hash);         
RemoveInternalTimer($hash);   
return undef;                 
}

Verstehe. Wie gesagt, im skalaren Kontext liefert "return;" das gleiche zurück wie return undef -> nämlich undef.
Mit meiner heutigen Brille sieht X_Undef so aus:

Code: (perl) [Auswählen]
sub X_Undef {
    my $hash = shift;
    DevIo_CloseDev($hash);
    RemoveInternalTimer($hash);
    return;                 
}

Das sollte sich 99.9% gleich verhalten und die Fälle wo es sich nicht gleich verhält deuten auf irgendein pathologisches Problem hin.

Keiner muss sich rechtfertigen. Ja - diesbezüglich schickt einen DevelopmentModuleIntro in die Wüste, aber die Doku hat ja auch schon einiges auf dem Buckel.
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

Offline Wzut

  • Developer
  • Hero Member
  • ****
  • Beiträge: 4544
Antw:PBP: return undef;
« Antwort #5 am: 28 März 2020, 17:45:19 »
@RichardCZ , Thema : my ( $hash, $name) = @_;    vs.   my $hash = shift; my $name = shift;
Da  hattest du IMHO die Woche was geschrieben (finde die Stelle jetzt nicht) die Grenze des Sinnvollen läge bei 3 x shift , bei mehr dann wieder @_
Stimmt das sinngemäß oder bilde ich mir jetzt wieder was ein ?
Und dann noch in dem Zusammenhang eine Variante die ich geerbt habe mir aber von der Optik gar nicht gefällt :
sub MAX_SerializeTemperature($)
{
  if (($_[0] eq  "on") || ($_[0] eq "off"))  { return $_[0]; }
  elsif($_[0] == 4.5)                        { return "off"; }
  elsif($_[0] == 30.5)                       { return "on"; }
  return sprintf("%2.1f",$_[0]);
}
Ich bin bei Schleifen zwar Fan von $_ , aber $_[0]  ?
Maintainer der Module: MAX, MPD, UbiquitiMP, UbiquitiOut, SIP, BEOK, readingsWatcher

Offline RichardCZ

  • Tester
  • Sr. Member
  • ****
  • Beiträge: 597
  • WHIP! HoBo war gestern.
    • Experimenteller FHEM Fork
Antw:PBP: return undef;
« Antwort #6 am: 28 März 2020, 19:00:27 »
Die 3 und mehr Argumente ist eher die Grenze um zu named Arguments überzugehen, weil positionale Parameter ihre Vorteile (Kürze/Würze) verlieren:

= @_; macht man eigentlich nur, wenn man tatsächlich eine Liste übergibt aber selbst das macht man nicht, weil man eine listreferenz übergeben sollte. (Um den Stack nicht immer vollzukleistern)

$_[0] macht nur dann Sinn, wenn man sich die übergebenen Argumente einmal anschauen möchte ohne sie vom Argumentstack zu holen. In etwa:

my $x = $_[0] ne 'bar' ? shift : 'mööp';     # hole nur wenn arg1 nicht 'bar' ist, ansonsten ist 'mööp' default
my $y = shift;                               # hole 1. oder 2. argument - je nachdem

also variable Parameteranzahl. $_[0] ist flott, aber wenn man das öfter als einmal macht, geht der Vorteil auch flöten.

sub MAX_SerializeTemperature($)
{
  if (($_[0] eq  "on") || ($_[0] eq "off"))  { return $_[0]; }
  elsif($_[0] == 4.5)                        { return "off"; }
  elsif($_[0] == 30.5)                       { return "on"; }
  return sprintf("%2.1f",$_[0]);
}


In diesem Fall ein simples shift und return post-ifs (merke if-elseif mit returns drin macht keinen Sinn). Und wenn "Du" (also Du als Erbe)  schon - richtig - ein Space zwischen if und der folgenden Klammer machst, dann doch bitte auch nach dem elsif. Ansonsten, PBP:

sub MAX_SerializeTemperature {
    my $temperature = shift;

    return $temperature if ($temperature =~ m{\A(on|off\z}xms);
    return 'off'        if ($temperature ==  4.5);
    return 'on'         if ($temperature == 30.5);

    return sprintf("%2.1f", $temperature);
}


Die ist jetzt von der Optik her besser, von der Logik her ist da noch einiges im Argen.

Wenn man mehr hätte - also angenommen man hätte 20 Werte nummern/regexen etc. die man auf andere Werte mappen möchte, dann muss man das Data-Driven machen. Hash oder Liste mit  wert => mapping, die man dann durchgeht.


Ich bin bei Schleifen zwar Fan von $_ , aber $_[0]  ?

Sobald Du verschachtelte Schleifen hast, ist es mit dem $_-Fandom schnell vorbei.
« Letzte Änderung: 28 März 2020, 19:03:15 von RichardCZ »
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

Offline Wzut

  • Developer
  • Hero Member
  • ****
  • Beiträge: 4544
Antw:PBP: return undef;
« Antwort #7 am: 28 März 2020, 19:22:38 »
THX
Zitat
weil man eine listreferenz übergeben sollte
da wirst du mal ein eigenes Kapitel für machen müssen, bis dahin stelle ich es hinten an.
Zitat
ein Space zwischen if und der folgenden Klammer machst, dann doch bitte auch nach dem elsif.
lach und ich dachte bisher immer ich mach es nur für mich weil es einfach schöner ausschaut mit Leerzeichen.
Zitat
m{\A(on|off\z}xms)
urgs und das für jemand der ohne https://regex101.com/ verloren ist .... :)
Zitat
man hätte 20 Werte nummern/regexen etc. die man auf andere Werte mappen möchte
trifft in dem Fall nicht zu , es geht immer um einen einzelen Wert.
Zitat
verschachtelte Schleifen hast, ist es mit dem $_-Fandom schnell vorbei
Habe ich auch schon bemerkt , aber die Äussere bekommt es :) 
Maintainer der Module: MAX, MPD, UbiquitiMP, UbiquitiOut, SIP, BEOK, readingsWatcher

 

decade-submarginal