Declaration compatible

[fnorm]

Mir ist aufgefallen, dass an einigen Stellen Parameter in vererbten Klassen verändert wurden, bzw fehlen.
Das führt dann zu einem PHP Warning: Declaration of ... should be compatible with ...

In der Produktivumgebung sollten zwar Warnings nicht ausgegeben werden, aber in meiner Lokalen Testumgebung hab ich das eigentlich Standardmäßig drin.

Den ein oder anderen hat ich in meiner Testumgebung schon behoben, aber mit dem letzten beta Update ist es natürlich wieder drin.

-----------------------
Das wäre

Code: Alles auswählen

 PHP Warning:  Declaration of wpsg_mod_weight::basket_save_product(&$data, &$product_data) should be compatible with wpsg_mod_basic::basket_save_product(&$data, &$product_data, $finish_order) in ...\wp-content\plugins\wpshopgermany\mods\wpsg_mod_weight.class.php on line 8

Behebung: Datei \mods\wpsg_mod_weight.class.php, Funktion: basket_save_product

Code: Alles auswählen

public function basket_save_product(&$data, &$product_data) {

ändern zu

Code: Alles auswählen

public function basket_save_product(&$data, &$product_data, $finish_order) {

Hat in dieser Funktion keinerlei Bedeutung, aber der Parameter finish_order wurde in Elternklasse hinzugefügt.
------------------------------------------------

Bei den folgenden Warnings wurden die Parameter teils inkompatibel geändert, also nicht einfach hinten dran, sondern z.b. ein zusätzlicher Paramter am Anfang, was prinzipiell schlecht ist.

Die fpdf Integration sollte man hier durch anders lautende Funktionen umsetzten, da die wpsg Funktionen auch was anderes machen als die orginalen fpdf.

[edit] Mir ist grad noch was aufgefallen. Die Klasse wpsg_fpdf gibt es doppelt. Einmal unter /mods/mod_rechnungen/wpsg_fpdf.class.php und nochmal unter /lib/wpsg_fpdf.class.php. Bis auf 2 Funktionen sind diese beiden gleich. Die aus mod_rechnungen beinhaltet noch Rotate und RotateText und 2 Variablendeklarationen am Anfang, dürfte also die aktuellere sein.

Code: Alles auswählen

PHP Warning:  Declaration of wpsg_fpdf::SetTextColor($strHEXCode) should be compatible with FPDF::SetTextColor($r, $g = NULL, $b = NULL) in  ...\wp-content\plugins\wpshopgermany\mods\mod_rechnungen\wpsg_fpdf.class.php on line 2

=> Vorschlag: Funktion umbenennen in SetTextHexColor($strHEXCode)

---------------------

Code: Alles auswählen

PHP Warning:  Declaration of wpsg_fpdf::Cell($w, $h, $text, $border, $ln, $align, $fill = 0) should be compatible with FPDF::Cell($w, $h = 0, $txt = '', $border = 0, $ln = 0, $align = '', $fill = false, $link = '') in ...\wp-content\plugins\wpshopgermany\mods\mod_rechnungen\wpsg_fpdf.class.php on line 2

Aktuell:

Code: Alles auswählen

    public function Cell($w, $h, $text, $border, $ln, $align, $fill = 0, $link) {
        parent::Cell($w, $h, $this->toIso($text), $border, $ln, $align, $fill);
    }

Ändern zu:

Code: Alles auswählen

    public function Cell($w, $h=0, $txt='', $border=0, $ln=0, $align='', $fill = 0, $link='') {
        parent::Cell($w, $h, $this->toIso($txt), $border, $ln, $align, $fill, $link);
    }

---------------------

Code: Alles auswählen

PHP Warning:  Declaration of wpsg_fpdf::MultiCell($x, $y, $h, $txt, $border = 0, $align = 'L', $fill = 0, $width = 0) should be compatible with FPDF::MultiCell($w, $h, $txt, $border = 0, $align = 'J', $fill = false) in ...\wp-content\plugins\wpshopgermany\mods\mod_rechnungen\wpsg_fpdf.class.php on line 2

=> Vorschlag: Funktion umbenennen in MultiCellWithPos(...)

zum Vergleich hier die fpdf Funktion

Code: Alles auswählen

MultiCell($w, $h, $txt, $border=0, $align='J', $fill=false)

hier die wpsg

Code: Alles auswählen

MultiCell($x, $y, $h, $txt, $border = 0, $align = 'L', $fill = 0, $width = 0)

Hier stimmen die Parameter gar nicht mehr überein, weil als erster Parameter einer hinzugefügt worden ist.

------------------------------------------------


Gut das hier php warnings ausgibt, hätte ich die Funktionen aus fpdf irgendwo im Rechnungstemplate gebraucht, hätte ich wahrscheinlich in der fpdf Doku nachgeschaut wegen der Parameter und wär wahrscheinlich nicht weit gekommen.

Noch eine Frage, wieso werft Ihr die Formatierung/Zeilenumbrüche raus im Quellcode? Das interne Kommentare rausfliegen ok, aber wieso die Formatierung? Ich arbeite in Netbeans und der kann Autoformat, also kein Problem das ganze wieder in eine einigermaßen passende Form zu bringen, aber das nach jedem Update in jeder Datei machen wo man was nachschauen will ist einfach unnötige Arbeit.

[maennchen1.de]

Hallo!
Danke für deine Mühe die du dir machst. Das haben wir hier sehr selten und wir wissen das sehr zu schätzen! Ich gebe mal die Fakten intern weiter und wir melden uns wieder!

[fnorm]

Reiner Eigennutz, ich denke ich werde mit Layout und den eigenen Anpassungen bis zum Release der nächsten Stable fertig sein, da spart mir das einiges an Arbeit wenn ich solche Sachen weitergebe und nicht bei jedem Beta-Update erneut korrigieren muss

Da fällt mir gleich noch was ein, da ich mit jQuery 2.x arbeite, muss ich da auch jedesmal einiges anpassen.
Ihr habt im Paket noch jQuery-validation-1.11.1, die nicht mit jQuery 2.x kompatibel ist, hab bei mir die 1.14.0 integriert, die läuft bis jetzt gut.
Allerdings hab ich diese Version nicht mit jQuery < 2.x ausprobiert.

Und noch eins.
Ich musste die Javascript Ladereihenfolge ändern. Allerdings passiert das laden bei euch in ./controller/wpsg_ShopController.class.php in der Funktion wp_enqueue().
Hier lade ich die frontend.js erst nach jquery, bzw in meinem Fall Bootstrap.
Hier auch die Änderung mit jQuery-validation-1.14.0.

Bin mir nicht sicher wie man das Konfigurierbar lösen könnte, aber frontend.js und jquery-validation-xxx sollte erst nach jquery geladen werden.

Code: Alles auswählen

wp_enqueue_script('wpsg_frontend', $this->getRessourceURL('js/frontend.js'));

sollte dann

Code: Alles auswählen

wp_enqueue_script('wpsg_frontend', $this->getRessourceURL('js/frontend.js'), array('jquery'));

und

Code: Alles auswählen

case '2': wp_enqueue_script('wpsg-validation', $this->getRessourceURL('js/jquery-validation-1.11.1/dist/jquery.validate.min.js'));
                        wp_enqueue_script('wpsg-validation-2', $this->getRessourceURL('js/jquery-validation-1.11.1/dist/additional-methods.min.js'));

sollte

Code: Alles auswählen

case '2': wp_enqueue_script('wpsg-validation', $this->getRessourceURL('js/jquery-validation-1.11.1/dist/jquery.validate.min.js'), array('jquery'));
                        wp_enqueue_script('wpsg-validation-2', $this->getRessourceURL('js/jquery-validation-1.11.1/dist/additional-methods.min.js'), array('jquery'));

heissen.

[MKJ]

Noch eine Frage, wieso werft Ihr die Formatierung/Zeilenumbrüche raus im Quellcode? Das interne Kommentare rausfliegen ok, aber wieso die Formatierung? Ich arbeite in Netbeans und der kann Autoformat, also kein Problem das ganze wieder in eine einigermaßen passende Form zu bringen, aber das nach jedem Update in jeder Datei machen wo man was nachschauen will ist einfach unnötige Arbeit.

Das sehe ich im Grunde auch so. Wenn jemand den Core klauen will, dann wird ihn die komprimierte Form auch nicht abhalten. Andererseits wird aber eben nur der Core komprimiert. Trotz Deiner sehr schönen Verbesserungsvorschläge muss man in der Regel am Core heute nichts mehr machen. Dennoch fände ich einen formatierten Core auch besser.

[fnorm]

Wenn du den Shop nicht um eigene Sachen erweiterst musst nichts mehr im Core ändern.

Bin zwar noch am Layouten usw., aber das ein oder andere bau ich noch mit ein, wo ich wahrscheinlich nicht drumherum komme auch im Core zu ändern. Prinzipiell auch kein Problem, da ich sowieso git verwenden, aber ich muss halt nach jedem Update für jede angepasste Datei rein, autoformat und dann meine Änderungen wieder mergen, sonst ist es einfach zu unübersichtlich.

[daschmi]

Hi,

danke für das gute und nützliche Feedback.

Ich habe eine Testversion 3.9.1.5276 erstellt, in der die Punkte berücksichtigt sind.

Viele Grüße

Daschmi