[Refactoring] Buienradar 3.0.6

Begonnen von Christoph Morrison, 22 April 2020, 20:54:08

Vorheriges Thema - Nächstes Thema

Christoph Morrison

Wie igami in https://forum.fhem.de/index.php/topic,110348.0.html, habe ich nun mal die ersten großen Brocken Sev. 3-5 von Perl::Critic in Buienrader behoben.
Wer Lust hat, darf gerne mal über den PR für 3.0.6 und natürlich den aktuellen Code schauen. Ich freue mich über jedes Feedback, gerne auch als Issue oder als PR.

RichardCZ

Daumen rauf für eigenen Namespace!

Nur ein paar Beobachtungen - sicher nicht vollständig.





given when -> for when
https://perldoc.perl.org/perlsyn.html#Switch-Statements

    if ( $opt eq 'rainDuration' ) {
        return ::ReadingsVal($name, 'rainDuration', 'unknown');
    }
    else {
        return qq[Unknown argument $opt, choose one of version:noArg startsIn:noArg rainDuration:noArg];
    }

    return;


Da ist mindestens ein return und ein else zuviel. (kommt häufiger vor)


    my ( $hash, $name, $opt, @args ) = @_;

    return qq['get $name' needs at least one argument] unless ( defined($opt) );


->

my $opt = shift // return qq{....




my $begin = $hash->{'.RainStart'};
return q[No data available] unless $begin;
return q[It is raining] if $begin == 0;


Ich vermute das erste return ist falls $begin undefined ist?
Schon mal die Meldung "it is raining" gesehen? Weil das unless $begin schlägt auch an wenn $begin 0 ist.

int(@a) == 2

->

@a == 2  ... aber bitte nicht @a

$a und $b sind magische, globale Variablen und man soll auch Homonyme vermeiden. (also wenn wir @list haben, sollte man nicht im gleichen lexical scope $list haben - es geht zwar, dient aber nicht der Übersichtlichkeit)




Du hast vermutlich mitbekommen, dass Buienradar (master) im -> HoBo aggregator ist?
https://gl.petatech.eu/root/HomeBot/-/commit/df9cf516cc87a252c49b01a3db3e794cf2051b40
wird begierig jedes Update schlürfen. ;-)
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

Christoph Morrison

Vielen Dank für deine Anmerkungen, ich habe die meistens bereits in Issues umgesetzt und werde nach 3.0.6. noch mal ein Review machen um die anderen Vorkommnisse zu finden.

Zitat von: RichardCZ am 22 April 2020, 21:25:10
Du hast vermutlich mitbekommen, dass Buienradar (master) im -> HoBo aggregator ist?
https://gl.petatech.eu/root/HomeBot/-/commit/df9cf516cc87a252c49b01a3db3e794cf2051b40
wird begierig jedes Update schlürfen. ;-)

Ja klar hab ich das gesehen. Webcount dagegen kannst du eigentlich weglassen, da gibt es keine Entwicklung mehr, ist im SVN auch in contrib gelandet.

RichardCZ

Zitat von: Christoph Morrison am 23 April 2020, 10:58:22
Vielen Dank für deine Anmerkungen, ich habe die meistens bereits in Issues umgesetzt und werde nach 3.0.6. noch mal ein Review machen um die anderen Vorkommnisse zu finden.

Ja klar hab ich das gesehen. Webcount dagegen kannst du eigentlich weglassen, da gibt es keine Entwicklung mehr, ist im SVN auch in contrib gelandet.

Jetzt habe ich erstmal nachgeschaut was das überhaupt ist.
https://www.wut.de/e-57652-ww-dade-000.php

So ein schönes gerätchen (ok, ein wenig teuer). Warum sollte man da den Support knicken?
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

Wzut

Zitat von: RichardCZ am 23 April 2020, 11:11:53
So ein schönes gerätchen (ok, ein wenig teuer). Warum sollte man da den Support knicken?
Ja die Geräte von WUT sind toll - mein Arbeitgeber hat da echt viele von im Netzwerk , 100% robust, eben kein Heimspielzeug sondern echte Industriequalität :) 
Maintainer der Module: MAX, MPD, UbiquitiMP, UbiquitiOut, SIP, BEOK, readingsWatcher

Christoph Morrison

Zitat von: RichardCZ am 23 April 2020, 11:11:53
So ein schönes gerätchen (ok, ein wenig teuer). Warum sollte man da den Support knicken?

Ist ein schönes Gerät, leider hab ich keines und aktuell auch keinen Einsatzzweck dafür (vulgo: Ich kauf mir kein's). Support wäre also eher theoretischer Natur, denn der alte Maintainer ist seit 10 Jahren oder so nicht mehr hier dabei.

RichardCZ

Ich habe darüber nachgedacht Hersteller bzgl. einer "Dauerleihstellung" anzuschubsen. Das wäre ja das Mindeste was sie tun könnten wenn sie ihr Zeug supported haben möchten. Bei z.B. der Linux Treiberentwicklung hat das in der guten alten Zeit auch funktioniert. (also Jahrtausendwende)

Heutzutage zahlt natürlich ein Hersteller dafür auch noch.
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.