Donnerstag, 31. Oktober 2019, 07:41
Leidenschaftlicher Programmierer
Aufgrund des lieben Kommentars von Jens habe ich mich dazu entschieden, auf den zugegeben nicht ganz so lieben Kommentar von Manni zu "antworten" - bzw. ganz allgemein zu dem Thema.
Ich bin seit meinem 13. Lebensjahr Programmierer. Und ich behaupte, dass ich von Anfang an mit Leidenschaft dabei bin (und mit Leidensbereitschaft, mit 14 war C++ dran).
Programmieren ist für mich keine Last, sondern eine wahnsinnig große Freude, die mich zugleich erfüllt und entspannt. Nachts oder früh morgens in (m)einem ruhigen Büro mit guter Instrumentalmusik zu sitzen und zu programmieren, ist für mich besser als jede Netflix-Session. Ich bin dann quasi glückselig.
In 99% der Fälle programmiere ich dabei "irgendwas" für manitu. Und ja, auch "CEO eines global agierenden Unternehmens" (Zitat Manni) tue ich das. Vielleicht auch gerade deswegen. Ich möchte "auch als CEO" nicht den Bezug zur Basis verlieren. Ich arbeite genauso im Support mit, und programmiere genauso mit. Bei fast jedem Projekt.
Ich gebe zu, dass ich gerade beim Programmieren sehr penibel bin. Da ich täglich sehe, was Code von tausenden anderen (z.B. die unzähligen Pakete in jeder Linux-Distribution) anrichten kann, lege ich Wert auf kleinste Details. Manchmal entscheiden diese kleinen Dinge über "Leben oder Tod".
Und genauso lege ich Wert auf Ressourcen sparenden Code. Ja, man muss Code nicht bis zum letzten Detail durchoptimieren, er soll v.a. für einen selbst und andere (!) lesbar sein. Alles, was der Lexer eh wegoptimiert, muss man nicht selbst optimieren.
Aber um einmal in dem konkreten Beispiel zu bleiben: Wenn man aus dem Paket php-iban z.B. nur die Funktion
Das wäre vielleicht nicht ganz so schlimm, wenn es eben nicht üblich wäre, solche Bibliotheken via Composer zu verwenden und auch aktuell zu halten. Somit wird die Datei in jedem Projekt, das die Bibliothek verwendet, mitgeladen. Beim Aufruf jeder PHP-Datei, die den Composer-Autoloader nutzt. Also vermutlich jeder.
Mein Patch bestand aus ganzen 3 Zeilen. Ich habe das Laden
Seht selbst:
Ich hatte gesehen, dass der Autor Patches nicht sehr aufgeschlossen ist, daher hatte ich meinen Patch so klein wie möglich gehalten. Quasi ohne Impact - auch optischer Natur
Normalerweise hätte ich, wie es auch in einigen Kommentaren hier bereits gesagt wurde, vorgeschlagen, die Datei in ein PHP-Code umzuwandeln und ggf. sogar noch bedingt via
Schade, dass der Autor dem Patch nicht sehr aufgeschlossen zu sein scheint. Vielleicht möchtet Ihr ihn ja nochmal mit Kommentaren im Pull-Request dazu animieren ?
Ich bin seit meinem 13. Lebensjahr Programmierer. Und ich behaupte, dass ich von Anfang an mit Leidenschaft dabei bin (und mit Leidensbereitschaft, mit 14 war C++ dran).
Programmieren ist für mich keine Last, sondern eine wahnsinnig große Freude, die mich zugleich erfüllt und entspannt. Nachts oder früh morgens in (m)einem ruhigen Büro mit guter Instrumentalmusik zu sitzen und zu programmieren, ist für mich besser als jede Netflix-Session. Ich bin dann quasi glückselig.
In 99% der Fälle programmiere ich dabei "irgendwas" für manitu. Und ja, auch "CEO eines global agierenden Unternehmens" (Zitat Manni) tue ich das. Vielleicht auch gerade deswegen. Ich möchte "auch als CEO" nicht den Bezug zur Basis verlieren. Ich arbeite genauso im Support mit, und programmiere genauso mit. Bei fast jedem Projekt.
Ich gebe zu, dass ich gerade beim Programmieren sehr penibel bin. Da ich täglich sehe, was Code von tausenden anderen (z.B. die unzähligen Pakete in jeder Linux-Distribution) anrichten kann, lege ich Wert auf kleinste Details. Manchmal entscheiden diese kleinen Dinge über "Leben oder Tod".
Und genauso lege ich Wert auf Ressourcen sparenden Code. Ja, man muss Code nicht bis zum letzten Detail durchoptimieren, er soll v.a. für einen selbst und andere (!) lesbar sein. Alles, was der Lexer eh wegoptimiert, muss man nicht selbst optimieren.
Aber um einmal in dem konkreten Beispiel zu bleiben: Wenn man aus dem Paket php-iban z.B. nur die Funktion
iban_to_machine_format() verwenden möchte, muss man eben die php-iban.php einbinden - und das liest automatisch die genannte registry.txt (quasi als Art von "main"-Funktion der Datei). Obwohl sie für diesen Funktions-Aufruf nicht verwendet wird.Das wäre vielleicht nicht ganz so schlimm, wenn es eben nicht üblich wäre, solche Bibliotheken via Composer zu verwenden und auch aktuell zu halten. Somit wird die Datei in jedem Projekt, das die Bibliothek verwendet, mitgeladen. Beim Aufruf jeder PHP-Datei, die den Composer-Autoloader nutzt. Also vermutlich jeder.
Mein Patch bestand aus ganzen 3 Zeilen. Ich habe das Laden
registry.txt einfach ausgelagert an die 2 einzigen Stellen, an denen sie nötig ist. Der Autor des Pakets hatte in seiner _iban_load_registry() bereits Code drin, der ein doppeltes Laden der Datei vermeidet.Seht selbst:
Ich hatte gesehen, dass der Autor Patches nicht sehr aufgeschlossen ist, daher hatte ich meinen Patch so klein wie möglich gehalten. Quasi ohne Impact - auch optischer Natur
Normalerweise hätte ich, wie es auch in einigen Kommentaren hier bereits gesagt wurde, vorgeschlagen, die Datei in ein PHP-Code umzuwandeln und ggf. sogar noch bedingt via
require_once() einzubinden, dann hätte der PHP-OPCache sie mitopitmiert.Schade, dass der Autor dem Patch nicht sehr aufgeschlossen zu sein scheint. Vielleicht möchtet Ihr ihn ja nochmal mit Kommentaren im Pull-Request dazu animieren
?
Kommentare
Ansicht der Kommentare: Linear | Verschachtelt
Jo
Aber Problem mit der vorgeschlagenen Lösung ist, da die zwei Funktionen jetzt Seiteneffekte bekommen. Das ist problematisch, da man z.B. eine neue Funktion einführen könnte, die das nutzt und den Call vergessen könnte oder in einem späteren refactoring nimmt man den "unnötig" Call raus und je nachdem in welcher Reihenfolge die Funktionen aufgerufen werden funktioniert es ... oder nicht.
Das "gut" zu machen wäre aber wohl in der Tat deutlich mehr Aufwand.
Manuel Schmitt
Es gibt genau 2 Funktionen, die sich darauf verlassen, dass die globale Variable gefüllt ist:
- _iban_country_get_info()
- iban_countries()
In beide Funktionen habe ich in meinen Patch genau das Laden aufgenommen, und den globalen "immer Laden"-Teil rausgenommen.
Das hat keine Seiteneffekte, und niemand muss beim Aufruf an irgendwas denken.
Und zu Composer: Da die IBAN-Funktionen in der composer.json der Lib
https://github.com/globalcitizen/php-iban/blob/master/composer.json
via
"autoload": {
"files": ["oophp-iban.php","php-iban.php"]
}
immer geladen werden, wird bei jeder PHP-Datei, die die Composer-Autoload-Funktion verwendet, die .txt-Datei der php-iban gelesen.
Schau nochmal genau hin
Fabian
Wer dennoch eine solche Sonderanpassung benötigt, wird wohl sich auch Gedanken darum gemacht haben, wie denn die Daten in das Array kommen...
onli
MK
Felix
So absurd ist die Frage ja nun nicht - denn während das Laden der globalen Variable bislang implizit war, muss man nach der Änderung daran denken, `_iban_load_registry` aufzurufen, wenn man beispielsweise eine neue Funktion einführt, die darauf zugreifen soll. Das ist nicht die Welt, aber genau genommen natürlich schon ein "maintenance burden". Schöner wäre vielleicht, die Registry nicht global zu machen, sondern beispielsweise als statische Variable in einer Funktion (`get_registry` o.ä.) zu verstecken.
Unabhängig davon scheint es mir aber zumindest schlechter Stil zu sein, andere Leute dazu zu animieren, auf Github Stimmung zu machen. Wäre ich der Maintainer, würde mich das wahrscheinlich eher davon abhalten, meine Entscheidung zu überdenken.
Jens
Eher würde es Sinn machen, den Speicherverbrauch zu vergleichen - hier liegt die Ersparnis aber quasi schon auf der Hand: Die größe der Textdatei + ein paar Headerbytes des Arrays. Gerade weil hier der Vorteil so offensichtlich auf der Hand liegt, ist es sehr fragwürdig warum darüber soviel diskutiert wird. In einem C++ Programm würde man auch nicht ohne Not einfach mal ein "new char[32768]" irgendwo ungenutzt rumgammeln lassen - aber so "unabstrakt" haben vermutlich nicht mehr viele programmiert, um sowas "automatisch" im Hinterkopf zu haben.
Stefan
Ich würde auch Messwerte fordern, bevor ich Code akzeptiere, der eine Performanceverbesserung verspricht. Jede Änderung ist Risiko, und manche Änderungen fordern zusätzlichen Wartungsaufwand für die Zukunft.
Und gerade Performance ist etwas, das auch gerne von Anfängern kommt: die haben dann irgendwo gelesen, dass - mal als Beispiel - Countdown-Schleifen effizienter seien, weil man da eine "decrement-and-jump-if-not-zero" Instruktion benutzen kann, und bauen dir alle for-Schleifen von ++i auf --i um. Im großen anonymen Internet sieht man halt nicht, ob der first-time-contributor ein solcher Anfänger ist. Es betrifft auch nicht nur Anfänger, ich hab hier durchaus schon nervenzehrend mit Leuten geredet, die länger im Geschäft waren, die mir einen 1000 Zeilen Patch reindrücken wollten und einfach nicht glauben wollten, dass eine gesparte Nanosekunde pro Iteration auf 100 Iterationen keine 10 Sekunden sparen kann.
Manuel Schmitt
In dem Fall war die Komplexität aber eh schon gegeben, da der Ursprungsautor ein "global $VARIABLE" überall dort, wo das gebraucht wird, gemacht hat (genau an 2 Stellen). An genau diesen 2 Stellen habe ich das Laden hinzugefügt.
Selbst bei Copy&Paste wird kein Fehler entstehen.
Und wer nur das global'en einer Variable im Kopf hat, der hat eh ein Problem...
Felix
Würde trotzdem dazu raten - gerade wenn der Maintainer einen konfrontativen Stil an den Tag legt - mit mehr Fingerspitzengefühl vorzugehen. Das erhöht die Chancen, dass der PR akzeptiert wird deutlich.