PMD-CPD Analyse nach Duplikaten, Codeverdopplungen, Copy & Pastes und so

Begonnen von RichardCZ, 11 April 2020, 09:16:05

Vorheriges Thema - Nächstes Thema

RichardCZ

https://pmd.github.io/latest/pmd_userdocs_cpd.html

Ich habe mal CPD, den Copy & Paste Detektor über FHEM gejagt.

Die Karl-Theodor Maria Nikolaus Johann Jacob Philipp Franz Joseph Sylvester Buhl-Freiherr von und zu Guttenbergs sind im Anhang aufgeführt.

Den Grund, warum man Duplikate vermeiden sollte spare ich mir an dieser Stelle, o.g. Link hat eine nette Zusammenfassung. Für mich kann ich nur sagen, das Dupletten im Code einer der größten Schandflecke sind von die wo's gibt. Ich wäre zutiefst betrübt wieder eine Diskussion zu erleben warum gerade in FHEM 1:1 Code-Duplikate ein Muss oder "schon in Ordnung" sind.

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

JoWiemann

#1
Besserwisser: Du sprichst von Dublette(n)?!

Wobei eine Dublette und ein Duplikat unterschiedliche Sachverhalte beschreiben.


Gesendet von iPad mit Tapatalk
Jörg Wiemann

Slave: RPi B+ mit 512 MB, COC (868 MHz), CUL V3 (433.92MHz SlowRF); FHEMduino, Aktuelles FHEM

Master: CubieTruck; Debian; Aktuelles FHEM

RichardCZ

Zitat von: JoWiemann am 11 April 2020, 09:58:18
Besserwisser: Du sprichst von Dublette(n)?!

Wobei eine Dublette und ein Duplikat unterschiedliche Sachverhalte beschreiben.

Ja, da ist das Englische "Dupe" mit mir durchgegangen.
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

rudolfkoenig

ZitatIch wäre zutiefst betrübt wieder eine Diskussion zu erleben warum gerade in FHEM 1:1 Code-Duplikate ein Muss oder "schon in Ordnung" sind.
Das wirst du von mir auch nicht hoeren, ich vermeide Duplikate, wo ich kann, stellenweise schon krankhaft.

Allerdings ist FHEM ein Gemeinschaftsprojekt, und aus Sicht eines Benutzers ist ein Modul, was Code kopiert, besser, als gar kein Modul. Das heisst nicht, dass ich das toll finde, ich weiss aber nicht, wie ich das ohne Maintainer abzuschrecken verbessern soll. Ich sehe den typischen Maintainer als den Familenvater, der ein Problem mit FHEM loesen kann, dafuer zwangsweise ein bisschen Perl lernt, aus Nettigkeit sein Code spendiert, aber nicht vor hat seine Familie fuer FHEM zu verlassen, und ein Perl-Guru will er auch nicht werden.

Ich mache mir sorgen, dass Deine unsanften Hinweise, auch wenn alleine betrachtet meist richtig sind, manche davon abhalten ein Modul zu veroeffentlichen. Nach dem Motto: lieber die Klappe halten, als ausgelacht werden.

Ich bin immer noch bei meinem urspruenglichen Vorschlag: mach Beispielmodule, und wir empfehlen diese als Grundlage fuer Neulinge.

herrmannj

Naja, für mich kann ich es beantworten. In GSI habe ich Code welcher funktioniert und den brauche ich auch in JsonMod. Weil Module self-contained sein sollen kopiere ich den halt rüber. Wenn wir das lockern sollten wäre ich happy den code auszulagern und in beiden Modulen zu verwenden. Um ehrlich zu sein weiß ich zwar nicht wo die Regel steht aber ich meine mich an Diskussionen zu erinnern und das Fazit war 'ein Modul, eine Datei'. Eventuell ist das aber gar nicht mehr aktuell (?)

RichardCZ

Zitat von: rudolfkoenig am 11 April 2020, 10:26:42
Familenvater, der ein Problem mit FHEM loesen kann, dafuer zwangsweise ein bisschen Perl lernt, aus Nettigkeit sein Code spendiert, aber nicht vor hat seine Familie fuer FHEM zu verlassen, und ein Perl-Guru will er auch nicht werden.

Ich mache mir sorgen, dass Deine unsanften Hinweise, auch wenn alleine betrachtet meist richtig sind, manche davon abhalten ein Modul zu veroeffentlichen. Nach dem Motto: lieber die Klappe halten, als ausgelacht werden.

Ich bin immer noch bei meinem urspruenglichen Vorschlag: mach Beispielmodule, und wir empfehlen diese als Grundlage fuer Neulinge.

Du willst Beispielmodule im gegebenen Rahmen. Das geht nicht, der Rahmen ist zu klein.
Ich kann aus einem Golf I keinen Veyron machen und einen Spoiler draufpappen und Streifen draufmalen betrachte ich nicht als "Beispielmodul".

Lass uns darüber reden den Rahmen an einigen Stellen zu verbessern, dann kann man auch wirklich auf bessere Beispiele hoffen:
https://forum.fhem.de/index.php/topic,110036.msg1040579.html#msg1040579
Was nicht bedeutet, dass die Anleitung wie man bessere FHEM Module auch in diesem Rahmen baut bereits erarbeitet wird:
https://forum.fhem.de/index.php/topic,110048.0.html (u.a.)




Diese Argumentation mit "Perl Guru" entlarvt an dieser Stelle die Diskussion als ein reines Sammelsurium von Schutzbehauptungen. Hier geht es nicht um Perl. Hier geht es um Softwareentwicklung im Allgemeinen. PMD-CPD unterstützt 20 oder 30 Sprachen, da ist Perl nur eine davon. Code Dpulikate sind in jeder Sprache ein Graus und zu vermeiden. Da gibt es auch nicht viel zu diskutieren.

Ich habe noch nie jemanden wegen seinem Code ausgelacht. Ich kann mich über Code lustig machen, nicht über die Person dahinter. Siehe
https://forum.fhem.de/index.php/topic,109511.0.html

Das Argument mit "besser als nix", "einem geschenkten Gaul" etc. verstehe ich. Was ich aber nicht verstehe, dass man bei FHEM in diesem zusammenhang die verquere Regel hat, dieser nette Amateurspender habe die alleinige Hoheitsgewalt über diesen seinen gespendeten GPL code(!) im Repo. Thema Janitors wieder. Das macht das FHEM-Entwicklungsmodell zu einem altbackenen "wir entwickeln nicht gemeinsam, sondern so nebeneinander her".

PS:

Und bitte, schiebt euch den "Perl Guru" sonstwohin. Perl macht 5% meines Skill Sets aus. Wer mich nur darauf reduzieren will, wird früher oder später seinen tiefenpsychologischen Aha-Effekt erleben.
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

RichardCZ

Zitat von: herrmannj am 11 April 2020, 10:52:40
Weil Module self-contained sein sollen kopiere ich den halt rüber.

Bitte was sollen die sein??? Was soll das überhaupt bedeuten? Alles in einem File oder was?

Dann macht natürlich das ~1MB EnOcean.pm "Sinn". Gibt es noch mehr so Regeln von denen man wissen sollte? z.B. nur in Unterhose programmieren oder so?
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

herrmannj

Zitat von: RichardCZ am 11 April 2020, 11:36:16
Bitte was sollen die sein??? Was soll das überhaupt bedeuten? Alles in einem File oder was?
Ja, alles in einem file.
Zitat
Gibt es noch mehr so Regeln von denen man wissen sollte? z.B. nur in Unterhose programmieren oder so?
Bist heute auf Stress gebürstet. :) Und ja, ich trage regelmäßig eine Unterhose beim programmieren. Aber hey, was immer Du willst (aber bitte, bitte, keine Details hier posten!!!)  :D :D :D


RichardCZ

Zitat von: herrmannj am 11 April 2020, 12:04:36
Ja, alles in einem file.

Super. Womit der Rahmen nochmal ein Stück kleiner geworden ist.
Und meine Anleitung hier: https://forum.fhem.de/index.php/topic,110048.0.html
ist demnach "laut den gegenwärtigen FHEM Regeln"(tm) fürn Arsch. Soviel zu "Beispielmodul".

Sorry. Ich kann ja verstehen wenn man vor 15 Jahren mal falsche Designentscheidungen und irgendwelche seltsamen (um nicht zu sagen: komischen)  Regeln aufgestellt hat. Weil man es entweder nicht besser wusste oder weil es damals(!) Sinn gemacht hat. Aber der Zeitpunkt ab dem ein Verteidigen dieser Anachronismen kafkaesk wird ist verdammt nah - wenn wir ihn nicht bereits überschritten haben.




Hey - warum wusste ich, dass wenn ich wieder ein Thema aufmache bei dem es nix zu Diskutieren gibt, dass dennoch eine Diskussion mit Nebenschauplätzen erfolgt? Muss irgendein FHEMVID-05 hier sein.

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

herrmannj

Zitat von: RichardCZ am 11 April 2020, 12:12:31
...
ist demnach "laut den gegenwärtigen FHEM Regeln"(tm) fürn Arsch. Soviel zu "Beispielmodul".
Kaum zu glauben dass es überhaupt Module gibt .. ;) So, aber jetzt chill mal ein wenig und genieß das Wetter !

Mittlerweile gibt es ja einige libs die den Weg durchaus geschafft haben. Mal schauen was der Rest zu eigenen libs sagt.

vg
Joerg

amenomade

Wie kann man =pod Text zwischen CommandRef DE/EN deduplizieren?

Siehe z.B.
Found a 58 line (405 tokens) duplication in the following files:
Starting at line 4250 of /data/proj/fhem/trunk/fhem/FHEM/32_WifiLight.pm
Starting at line 4895 of /data/proj/fhem/trunk/fhem/FHEM/32_WifiLight.pm

Found a 94 line (305 tokens) duplication in the following files:
Starting at line 4746 of /data/proj/fhem/trunk/fhem/FHEM/42_SYSMON.pm
Starting at line 5324 of /data/proj/fhem/trunk/fhem/FHEM/42_SYSMON.pm

Found a 102 line (479 tokens) duplication in the following files:
Starting at line 1145 of /data/proj/fhem/trunk/fhem/FHEM/44_S7.pm
Starting at line 1251 of /data/proj/fhem/trunk/fhem/FHEM/44_S7.pm
Pi 3B, Alexa, CUL868+Selbstbau 1/2λ-Dipol-Antenne, USB Optolink / Vitotronic, Debmatic und HM / HmIP Komponenten, Rademacher Duofern Jalousien, Fritz!Dect Thermostaten, Proteus

RichardCZ

Zitat von: amenomade am 11 April 2020, 14:56:46
Wie kann man =pod Text zwischen CommandRef DE/EN deduplizieren?

Siehe z.B.

Found a 94 line (305 tokens) duplication in the following files:
Starting at line 4746 of /data/proj/fhem/trunk/fhem/FHEM/42_SYSMON.pm
Starting at line 5324 of /data/proj/fhem/trunk/fhem/FHEM/42_SYSMON.pm


Also selbst bei den derzeit verfügbaren bescheidenen Bordmitteln würde ich ja sowas wie

Beispiele siehe <a href="commandref_EN.html#SYSMON">EN Doku</a> machen.
Und diese eben nur an einer Stelle pflegen.

Wo das nicht geht, geht's halt nicht. Dann müsste man sich überlegen, ob =pod in einem multilingualen Umfeld die richtige Wahl ist.
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

zap

Ich bin beruhigt. Die Dupletten in meinen Modulen könnt ihr ignorieren. Da wurde das veraltete RPC Modul mit dem neuen verglichen => grüne Tonne.
Wundert mich, dass da nicht mehr doppelt ist.
2xCCU3, Fenster, Rollläden, Themostate, Stromzähler, Steckdosen ...)
Entwicklung: FHEM auf AMD NUC (Ubuntu)
Produktiv inzwischen auf Home Assistant gewechselt.
Maintainer: FULLY, Meteohub, HMCCU, AndroidDB

dev0

#13
Zitat von: RichardCZ am 11 April 2020, 11:32:43
Was ich aber nicht verstehe, dass man bei FHEM in diesem zusammenhang die verquere Regel hat, dieser nette Amateurspender habe die alleinige Hoheitsgewalt über diesen seinen gespendeten GPL code(!) im Repo.

Ich muss ein wenig ausholen:

Ich bin Ersteller des 34_ESPEasy.pm FHEM Modules und des entsprechenden ESPEasy Firmware Controller Plugins. Und: nein, ich bin kein Programmierer oder Softwareentwickler, ich habe mir ein wenig Perl und C Syntax angeeignet um meine Bedürfnisse in FHEM umsetzen zu können.

Ich würde mich darüber freuen, wenn neben mir noch jemand meine Module betreuen würde (Code verbessern, an aktuelle Firmware Version anpassen, etc.). Derjenige müßte die Änderungen allerdings auch supporten bzw. ggf. zurück drehen, wenn es Probleme gibt. Es gibt ja zur Zeit leider keinen Test/Dev Zweig im svn...

Nachdem mein Pull Request des FHEM Controllers in das ESPEasy Projekt eingeflossen ist, ist das Plugin mehrfach von unterschiedlichen Mitstreitern angepasst und optimiert worden. Das finde ich toll und sinnvoll und es gab bisher nie Probleme und wenn, dann wäre das nach x Issues auf Github zurück gezogen oder gefixed worden, da bin ich mir recht sicher.




Code Duplikate und wiederverwendbaren Code erstellen:

Ich habe das ESPEasy Modul so ausgelegt, dass der Anwender die Möglichkeit hat Zugriffe auf IP Adressen/Bereich zu beschränken. Eine Möglichkeit wäre gewesen, die FHEM interne und regex basierte Variante zu benutzen. Da ich das als Netzwerker und Anwender als unbefriedigend befand, habe ich einen eigenen access controll Mechanismus eingebaut, der die Eingaben von IPs, Komma separierten Listen, Netmasks (bitmask oder dotted decimal) oder regex erlaubt und umgehe dabei den FHEM internen Mechanismus.
Klar, man jetzt sagen, warum nicht einen Patch an den Core Entwickler schicken und hoffen, dass es eingebaut wird. Mit Sicherheit ist mein Code weder optimiert, zu teuer (lahm) und nicht auf Angriffe getestet, um ihn in allen Modulen zu verwenden. Das kann ich als "Anweder" auch nicht leisten.
[EDIT:] Ggf. wäre es viel sinnvoller gewesen nach einem entsprechen CPAN Modul zu suchen, dass das viel besser kann, aber auch da ist die jetzige FHEM Policy mMn nicht sehr sinnvoll: Mach alles alleine mit möglichst wenig Abhängigkeiten... Zumindest hatte ich das damals so im Kopf.

Mein Gedanke war ursprünglich mal den Code in eine NetUtil.pm oder so auszulagern, um ihn Anderen zugänglich zu machen. Aber dann wäre ich, nach jetzigen Vorgehen (eine Datei - ein Maintainer), der Maintainer dafür gewesen und hätte die Verantwortung dafür übernehmen müssen. Das wollte und hätte ich nicht leisen können. Ganz abgesehen von dem bashing "wer braucht den sowas -  das können wir auch jetzt schon alles mit einer regex definieren".

Ich fände es gut die Prinzipien von "ein Modul -> eine Datei" und "ein Modul und niemand anderes geht daran" aufzuweichen und zu überdenken.

Was könnten die Nachteile sein, wenn "globale" Entwickler Code von anderen anfassen? Klar, irgendetwas funktioniert nicht mehr und der "globale Entwickler" hat nicht die Möglichkeit das nachzuvolluiehen, weil er zB. die Hardware nicht hat, die sich buggy verhält. Dann muss das zurück gedreht werden, aber viel besser __vorher__ in einem eigenen Branch vorab zur Verfügung gestellt werden. Auch wenn das bisher nicht so funktioniert hat wie es sollte...




Wie dem auch sei:

Selbst wenn @RichardCZ eine Heuschrecke seien sollte, die nach x Wochen/Monaten/Jahren weiter zieht, wäre mMn etwas positves in FHEM bewegt worden, wenn man darauf eingeht und seine Art und Weise beiseite läßt. Die Art und Weise von einigen alten Hasen ist teilweise auch recht hart und läßt Hobbyisten erstarren.

RichardCZ

Zitat von: dev0 am 12 April 2020, 11:27:12
Was könnten die Nachteile sein, wenn "globale" Entwickler Code von anderen anfassen? Klar, irgendetwas funktioniert nicht mehr und der "globale Entwickler" hat nicht die Möglichkeit das nachzuvolluiehen, weil er zB. die Hardware nicht hat, die sich buggy verhält. Dann muss das zurück gedreht werden, aber viel besser __vorher__ in einem eigenen Branch vorab zur Verfügung gestellt werden. Auch wenn das bisher nicht so funktioniert hat wie es sollte...

Meine - wiederholte - Anspielung auf das "Janitors"-Konzept des Linux-Kernels geht ja noch nicht einmal so weit wie "Globaler" Entwickler.
Die Aufgabe eines Janitors ist es Code sauber zu machen (Stichwort: Refactoring, Dokumentation) und das beinhaltet explizit NICHT Änderungen an der Funktionalität.

D.h. ja, ohne Testsuite kann es natürlich schon mal zu Regressionen kommen, aber das sollte die Ausnahme und nicht die Regel sein, denn ein Janitor geht ja mit der Maßgabe an den Code eben nix funktional zu verändern. Ergo sollte (sollte!) eine Umformatierung des Code, perlcritic/PBP Anpassungen, Namespace Aufräumarbeiten etc. gar nicht am User ankommen.

Leider - und da muss man als Janitor auch sehr diszipliniert sein - ist es nicht nur "nix funktional ändern", sondern manchmal sogar: Behalte den gegenwärtigen Fehler bei obwohl Du es besser weist.

Ich habe z.B. an etlichen Stellen ein

my $x = shift || return 'blah';

lassen müssen, obwohl eindeutig ist, dass man

my $x = shift // return 'blah';

meinte. FHEM behandelt an ungefähr 1000 Stellen eine Eingabe von 0 "als gäbe es sie nicht", aber das zu korrigieren ist eigentlich schon knapp hinter der Grenze des Wirkbereichs eines Janitors.


PS: Eine Heuschrecke, die erst nach Jahren weiterzieht ist IMHO keine Heuschrecke mehr.

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