Captcha Klasse wird unnötig geladen und initialisiert

[Gulliver72]

Hallo zusammen,

ich bin über folgendes gestolpert

In der /media/content/contact_us.php wird in den Zeilen 25-36 die Captcha Klasse eingebunden und initialisiert.
Das erfolgt unabhängig davon, ob das Captcha genutzt wird oder nicht.
Nun habe ich das angepasst, damit das nur bei Nutzung erfolgt

bisher

Code: PHP  [Auswählen]

  // include needed classes
  require_once(DIR_WS_CLASSES.'modified_captcha.php');
 
  $mod_captcha = $_mod_captcha_class::getInstance();
   
  // captcha
  $use_captcha = array('contact');
  if (defined('MODULE_CAPTCHA_ACTIVE')) {
    $use_captcha = explode(',', MODULE_CAPTCHA_ACTIVE);
  }

  defined('MODULE_CAPTCHA_CODE_LENGTH') or define('MODULE_CAPTCHA_CODE_LENGTH', 6);
  defined('MODULE_CAPTCHA_LOGGED_IN') or define('MODULE_CAPTCHA_LOGGED_IN', 'True');

neu

Code: PHP  [Auswählen]

  // captcha
  $use_captcha = array('contact');
  if (defined('MODULE_CAPTCHA_ACTIVE')) {
    $use_captcha = explode(',', MODULE_CAPTCHA_ACTIVE);
  }
  if (in_array('contact', $use_captcha) && (!isset($_SESSION['customer_id']) || MODULE_CAPTCHA_LOGGED_IN == 'True')) {
    defined('MODULE_CAPTCHA_CODE_LENGTH') or define('MODULE_CAPTCHA_CODE_LENGTH', 6);
    defined('MODULE_CAPTCHA_LOGGED_IN') or define('MODULE_CAPTCHA_LOGGED_IN', 'True');

    require_once (DIR_WS_CLASSES.'modified_captcha.php');

    $mod_captcha = $_mod_captcha_class::getInstance();
  }  

Ist die dabei genutzte erste Zuweisung überhaupt richtig??

Code: PHP  [Auswählen]

$use_captcha = array('contact');

Wäre das nicht korrekter?

Code: PHP  [Auswählen]

$use_captcha = array();

Desweiteren sollte die Zeile 69 geändert werden

Code: PHP  [Auswählen]

      if ($mod_captcha->validate($_POST['vvcode']) !== true) {

Das ist in der /create_account.php zur Vermeidung eines NOTICE bereits angepasst

Code: PHP  [Auswählen]

    if ($mod_captcha->validate((isset($_POST['vvcode'])) ? $_POST['vvcode'] : '') !== true) {

Das Einbinden und Initialisieren der Captcha Klasse betrifft auch die
create_account.php
create_guest_account.php
newsletter.php
password_double_opt.php
product_reviews_write.php

Leider sind in den Dateien die Zeilen teilweise verstreut.

Außerdem ist in dem Zusammenhang auch darüber nachzudenken, ob es sinnvoll ist, folgenden Codeblock in der /includes/classes/modified_captcha.php vor die Klasse zu setzen.

Code: PHP  [Auswählen]

foreach(auto_include(DIR_FS_CATALOG.'includes/extra/captcha/','php') as $file) require_once ($file);

$_mod_captcha_class = CAPTCHA_MOD_CLASS;
if (!class_exists($_mod_captcha_class)) {
  $_mod_captcha_class = 'modified_captcha';
}

Die hier includierten Captcha-Klassen erben von der Klasse modified_captcha, welche erst danach im Code folgt. Das ist an sich kein Problem. Wenn aber jemand aus irgendwelchen Gründen bereits hier eine Initialisierung durchführt, läuft das auf einen Fatal Error. Deshalb schlage ich vor, den Codeblock in der Datei hinter die Klasse zu schieben.  Somit kann dann jederzeit eine Initialisierung der eigenen Klasse erfolgen. Damit ist die Erb-Logik sauber. Ich habe in der Klasse auch nichts gefunden, was dem widerspricht.

Was denkt ihr darüber?

VG Bert

Linkback: https://www.modified-shop.org/forum/index.php?topic=41179.0

[noRiddle (revilonetz)]

Hi Gulliver72.

Den ersten Teil deiner Ausführungen sehe ich genauso wie du.
Den zweiten Teil bzgl. des Codes in der modified_captcha Klasse halte ich nicht für so relevant.
Der Sinn des auto_include hier ist ja lediglich die Klasse zu erweitern und nicht eine Klasse zu bauen die man extra, wie du sagst, "bereits hier" (wo eigentlich ?),  instanziieren möchte.
Wenn es in modified irgendwann mal eine Autoload-Funktion geben sollte wäre die Code-Verlagerung unter die Klasse nach deinem Vorschlag allerdings nötig, weil eine Class-Extension in diesem Fall nach ihrer Elternklasse definiert werden muß.

Übrigens muß man seine Klasse in /includes/extra/captcha/ auch gar nicht als Extension der modified_captcha-Klasse bauen, sie kann auch eigenständig sein und dann kannst du sie auch "bereits hier" instanziieren.
(Heißt übrigens nicht initialisieren, wobei meine Wenigkeit instanziieren auch schon verkehrt als instantiieren geschrieben hat... ).

Gruß,
noRiddle