Autor Thema: 93_DbRep.pm  (Gelesen 642 mal)

Offline RichardCZ

  • Tester
  • Sr. Member
  • ****
  • Beiträge: 597
  • WHIP! HoBo war gestern.
    • Experimenteller FHEM Fork
93_DbRep.pm
« am: 15 April 2020, 17:56:43 »
Ab Zeile https://svn.fhem.de/trac/browser/trunk/fhem/FHEM/93_DbRep.pm#L2204

und die darauf folgenden ~ 300 Zeilen fühle ich mich an https://de.wikipedia.org/wiki/Brutalismus
erinnert.

Ehrlich, aber mit hohem Materialeinsatz. Die Redundanz schlägt einem förmlich ins Gesicht:

if (AttrVal($name,"timestamp_begin","") eq "current_year_begin" ||
          AttrVal($name,"timestamp_end","") eq "current_year_begin") {
     $tsbegin = strftime "%Y-%m-%d %T",localtime(timelocal(0,0,0,1,0,$year)) if(AttrVal($name,"timestamp_begin","") eq "current_year_begin");
$tsend   = strftime "%Y-%m-%d %T",localtime(timelocal(0,0,0,1,0,$year)) if(AttrVal($name,"timestamp_end","") eq "current_year_begin");
 }
 
 if (AttrVal($name, "timestamp_begin", "") eq "current_year_end" ||
          AttrVal($name, "timestamp_end", "") eq "current_year_end") {
     $tsbegin = strftime "%Y-%m-%d %T",localtime(timelocal(59,59,23,31,11,$year)) if(AttrVal($name,"timestamp_begin","") eq "current_year_end");
$tsend   = strftime "%Y-%m-%d %T",localtime(timelocal(59,59,23,31,11,$year)) if(AttrVal($name,"timestamp_end","") eq "current_year_end");
 }
 
 if (AttrVal($name, "timestamp_begin", "") eq "previous_year_begin" ||
          AttrVal($name, "timestamp_end", "") eq "previous_year_begin") {
     $tsbegin = strftime "%Y-%m-%d %T",localtime(timelocal(0,0,0,1,0,$year-1)) if(AttrVal($name, "timestamp_begin", "") eq "previous_year_begin");
$tsend   = strftime "%Y-%m-%d %T",localtime(timelocal(0,0,0,1,0,$year-1)) if(AttrVal($name, "timestamp_end", "") eq "previous_year_begin");
 }

etc. ad inf.
 

Das ist ein Code, den man auf 1/10 zusammenschrumpfen kann und er wird dadurch schneller und wartbarer und weniger fehleranfällig.
Als erstes könnte man die AttrVal - lookups in einer Variable zwischenspeichern, weil man hier pro if-Zweig immer und immer wieder
AttrVal aufruft für immer und immer wieder die selbe Abfrage.

Vorschlag - 1. Schritt:

my $avtsbegin = AttrVal($name, 'timestamp_begin', q{});
my $avtsend   = AttrVal($name, 'timestamp_end',   q{});

if ('current_year_begin' =~ m{\A($avtsbegin|$avtsend)\z}xms) {
    my $ts = strftime "%Y-%m-%d %T", localtime( timelocal( 0, 0, 0, 1, 0, $year ) );
    $tsbegin = $ts if ($avtsbegin eq "current_year_begin" );
    $tsend   = $ts if ($avtsend   eq "current_year_begin" );
}


Wobei die ersten zwei Zeilen nur am Anfang dieses 300 Zeilen Marathons gebraucht werden.
Wir sparen uns eine Menge Code und einiges an lookups, auch die localtime(timelocal) Duplizität verschwindet.

Dann stellen wir fest, dass sich die ganzen if-Zweige eigentlich logisch ausschliessen, also wäre das ein Fall für einen if-elsif Bandwurm gewesen.
Aber selbst dieser Bandwurm wäre besser gewesen, weil man entsprechende AttrVal Aufrufe nur bis zu einem Match gemacht hätte und nicht Toujours.

Und wir stellen fest, dass auch der im 1. Schritt geschrunkene Code noch eine Menge an Redundanz enthält. Z.B. gibt es einen Vergleichsstring der da 3 x pro if-Zweig vorkommt.
Aber ich will da nicht zuviel abladen. Wenn Interesse ist, können wir gemeinsam schauen, wie man diesem 900kB Oschi eine Fitness-Kur verpassen könnte.
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

Offline DS_Starter

  • Developer
  • Hero Member
  • ****
  • Beiträge: 7504
Antw:93_DbRep.pm
« Antwort #1 am: 15 April 2020, 18:47:10 »
Hallo Richard,

ja ich kenne diese und noch ein paar andere Stellen (nicht nur in DbRep)
die ich mir nochmal vornehmen wollte. Aber ich gehe schrittweise vor und versuche erstmal in meinen Modulen die Empfehlungen bezüglich Prototypes und wenn möglich auch Packages umzusetzen.
Und vor allem auch fehlfrei, weil ich viel Zeit in Usersupport investiere ist es mir wichtig vorsichtig zu agieren und lieber dreimal hinzuschauen und zu testen.

Also ... kommt alles mal dran, aber der Reihe nach und ich komme dann auch gerne auf dein Unterstützungsangebot zurück.

Danke und Grüße,
Heiko
ESXi 6.5 @NUC6i5SYH mit FHEM auf Debian 10, DbLog/DbRep mit MariaDB auf VM
Maintainer: SSCam, SSChatBot, SSCal, SSFile, DbLog/DbRep, Log2Syslog, SolarForecast,Watches, Dashboard
Kaffeekasse: https://www.paypal.me/HMaaz
Contrib: https://svn.fhem.de/trac/browser/trunk/fhem/contrib/DS_Starter

Offline RichardCZ

  • Tester
  • Sr. Member
  • ****
  • Beiträge: 597
  • WHIP! HoBo war gestern.
    • Experimenteller FHEM Fork
Antw:93_DbRep.pm
« Antwort #2 am: 15 April 2020, 18:52:07 »
Kein Problem, der Beispielcode kann ja so lange in der Schublade hängen bis man ihn braucht.

Wo wäre der ganze Spaß hin, wenn wir schon morgen nix mehr zu verbessern hätten?  ;)

Und vor allem: Solche kurzen Artikel von mir sind ja als Vorschlag und mögliche Inspiration für alle Zuschauer gedacht.
Keiner muss sich dadurch "veräppelt" oder "getrieben" fühlen.

Aber ich denke den Modus Operandi bekommen wir so langsam hin - oder?. *daumenrauf*
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.
Gefällt mir Gefällt mir x 4 Liste anzeigen