[Erledigt] OOP: Problem mit Funktionsauswertung

Baltasar · 30.01.2011, 12:44

Mahlzeit,

ich programmiere schon was länger in PHP, aber meist nur kleinere Sachen und die ich habe stets imperativ gelöst. Nun möchte ich aber mein Wissen erweitern und auch in PHP in die Welt der OOP eindringen.

Daher programmiere ich der Zeit ein User-System mit Registratur und Login und allem drum und dran. Ich sage gleich vorweg: Ich weiß, dass es etliche Frameworks und Scripte gibt, die das 1000x besser gelöst haben als ich es gerade versuche. Ich will nicht das Rad neu erfinden. Ein Produktiveinsatz ist nicht geplant. Es geht nur darum, dass ich so am besten Lerne.

Auf jeden Fall ist der Ablauf wie folgt:

Man registriert sich über ein übliches Formular (Username, Passwort, Email). Anschließend wird der Username überprüft. Wenn nicht: Datenbankeintrag und Aktivationsmail. Wenn doch: Nichts davon und ein entsprechender Hinweis.

Das Problem ist nun, dass ich immer den Hinweis bekomme, dass der Name belegt ist, auch wenn es nachweislich nicht der Fall ist. Und Trotz der Fehlermeldung laufen alle Prozesse im Hintergrund (Datenbankeintragung etc.) fallunterscheidend richtig. Es geht mir nur um die Meldung.

Der Prozess geht von der register.php aus. Dort gibt es folgende If-Kontrolle:

PHP-Code:

      if (isset($_POST['username'])) {

        $user = new User($_POST['username'], sha1($_POST['password']), $_POST['email']);
        if ($user->register() == true) {
            echo('Registration erfolgreich. Ihnen wurde ein Aktivierungslink an die angegebene Adresse geschickt. ');
        } else 
         {
            echo('Username wird leider schon verwendet. ');
        }
    }

Die Klasse User liegt in der class.User.php und der entscheidende Teil sieht wie folgt aus. Dort wird ein neues Objekt vom Typ registration erzeugt und der User wird als Parameter übegeben.

PHP-Code:

      public function register() {

        $this->registration = new Registration($this);
        if ($this->registration->controlInput() == false) {return false;}
        return true;

    }

Die Klasse Registrations liegt in der class.Registration.php. Dort sieht der entscheidende Teil (die Methode controlInput() ) so aus:

PHP-Code:

          public function controlInput() {
        
            $findExistingUser = "SELECT `username` FROM `user` WHERE `username` = '" . $this->user->getUsername() . "'";
            $executeSql = mysql_query($findExistingUser) or die(mysql_error());
        
            while($row = mysql_fetch_object($executeSql)) {
                return false;    
            } 
        
        return true;        
        
        }

Der Konstruktor der Registration sieht wie folgt aus (und liegt natürlich ebenfalls in der Klasse)

PHP-Code:

          public function __construct($pUser) {
            $this->user = $pUser;
            if ($this->controlInput() == true) {
                $this->setSQL();
                $this->activationMail();    
            }
        }

Um das Problem nochmal zusammenzufassen: Ich erzeuge bei der Registration ein Objekt "user". Es wird die Methode "register()" aufgerufen, die ihrerseits ein Objekt vom Typ "Registration" erzeugt und die Kontrolle (ob der Username schon vergeben ist) mittels "controlInput()" vornimmt. Der Kontruktur fragt das Ergebnis ab. Nur im Falle von "true" wird mittels "setSQL()" und "activationMail()" der User erstellt. Die Idee war, dass nun im Falle von "false", dieses "false" durch die Methoden hinweg abgefragt und weitergereicht wird. Aber anscheinend ist da irgendwo der Wurm drin.

Ich weiß, dass das nun eine Menge Infos waren und hoffen, dass jemand da mal drüber gucken kann. Ich habe mir Mühe gegeben wie wirklich entscheidenden Code-Fragmente rauszusuchen.

Beste Grüße und schon mal vielen Dank,

Der Baltasar

curryworld · 30.01.2011, 13:00

gibt dir denn $this->user->getUsername() den richtigen Usernamen zurück?

und warum testest du mit einer Schleife und nicht mysql_num_rows?

Chriz · 30.01.2011, 13:16

Hallo,

ob ein Benutzername existiert oder nicht prüfst du mit SELECT COUNT(*) FROM user WHERE `username` = '<USERNAME>'. Das Ergebnis, das immer existiert, holst du mit mysql_result() ab. Eine Schleife brauchst du nicht.

Wenn es 0 ist, ist der Benutzername noch frei.

Übrigens ist dein Script anfällig für SQL-Injections, benutz mysql_real_escape_string() um das zu verhindern.

Deine Kopplung von Datenbezug, Registrierung und "controlInput" finde ich auch nicht sehr gut, trenn doch die Zuständigkeiten etwas besser.

Baltasar · 30.01.2011, 14:00

Ich weiß, dass das Script (noch) anfällig für Injections ist. Ich wollte erstmal das Gerüst ans laufen kriegen. Danke aber für den Hinweis

Den Wiki-Artikel werde ich mir nachher auch noch zu Gemüte führen. Danke auch dafür.

Nun habe ich die "controlInput()" wie folgt überarbeitet. Am Ergebnis ändert sich trotzdem nichts.

PHP-Code:

          public function controlInput() {
        
            $findExistingUser = "SELECT COUNT(*) FROM `user` WHERE `username` = '" . $this->user->getUsername() . "'";
            $executeSql = mysql_query($findExistingUser) or die(mysql_error());
        
            if (mysql_result($executeSql,0,0) != 0){
                return false;    
            } 
        
        return true;        
        
        }

"$this->user->getUsername()" habe ich gerade mal testweise ausgeben lassen und es liefert auch den richtigen Usernamen zurück.

Evtl. hat noch jemand ne Idee?

Chriz · 30.01.2011, 15:09

Zitat:

Zitat von Baltasar

Ich weiß, dass das Script (noch) anfällig für Injections ist. Ich wollte erstmal das Gerüst ans laufen kriegen. Danke aber für den Hinweis

Und dann im Nachhinein alle Skripte wieder überarbeiten? Erfahrungsgemäss wird das nicht gemacht. Es bedeutet quasi 0-Aufwand mehr, das sofort zu machen. Alles andere ist Schlamperei, auch wenn es nur für dich selbst ist. Gerade zu Testzwecken kann man es ja ruhig mal richtig machen.

PHP-Code:

  public function controlInput()
{
  $username = $this->user->getUsername();
  $sql = "SELECT COUNT(*) FROM `user` WHERE `username` = '" . mysql_real_escape_string($username) . "'";
  $res = mysql_query($sql) or die(mysql_error());
  $count = mysql_result($res, 0, 0);
  return $count == 0;
}

controlInput() erklärt überhaupt nicht, was die Methode macht. isUniqueUser() wäre doch treffender, oder?

Bitte lern auch mal zu debuggen, du stocherst ja komplett im Nebel. var_dump() solltest du immer für alle relevanten Variablen einsetzen.

Baltasar · 30.01.2011, 15:49

Eigentlich heißt das Ding controlInput(), da später auch die Email-Adresse analog zum Usernamen geprüft werden soll. In einem Abwasch sozusagen. Ich hänge halt aktuell nur schon beim Usernamen.

var_dump() bringt mich gerade auch nicht weiter, werde aber ab jetzt einsetzen.

Naja...mal gucken.

Edit: Zur Überarbeitung mit Injections: Methoden, die schon fertig sind (setSql zu, Beispiel) sind bereits mit "mysql_real_escape_string" versehen. Ich mache das immer Abschnittsweise.

tr0y · 30.01.2011, 15:57

Dein Klassen-Design ist ein wenig ... naja

Datenbank:

PHP-Code:

  class DatabaseAction_RegisterUser extends DatabaseAction {
   public function Register(User $user) {
      $result = self::$sqlHandle->Query(/* SQL Query * /);
      /* error processing */
      return ...;
   }

   public function isRegistred(User $user) {
      $result = self::$sqlHandle->Query(/* SQL Query */);
      /* error processing & return von true oder false */
   }

   public function isRegistrationAllowed() {
      /* error processing & return von true oder false */
      $result = self::$sqlHandle->Query(/* SQL Query */);
   }
}

class DatabaseAction_GetUser extends DatabaseAction {
   public function getUserdataByID($id) { ... }
   public function getUserdataByEmail($id, limit=false) { ... }
   public function getUserdataByState($id, limit=false) { ... }
   /* ect.. pp */
}

abstract class DatabaseAction {
   protected static $sqlHandle;
   public static function CreateHandle($server,$user,$pass,$database) {
      self::$sqlHandle = new mysqli(...);
      /* fehler behandlung und return */
   }
}

User Related Class:

PHP-Code:

  class UserFactory {

    public function Registrate($arrayOfUserdata) {
      $view = new DatabaseAction_RegisterUser();
      $user = $this->createUserObjectByArray($arrayOfUserdata);
      if ( $view->isRegistred($user) ) { 
         /* ... */
         return false;
      }
      else {
         /* ... */
         return $this->createUserObjectAfterRegister($id);
      }
    }

    public function Generate($userid) {
       return $this->createUserObjectByUserID($userid);
    }

    public function Delete(User $user) {
       /* ... */
    }

    // ---

    protected function createUserObjectByArray($array) {
       /* ... */
       $obj = new User();
       $obj->blub = "bla"; // ...
    }

    protected function createUserObjectByUserID($id) {
       /* ... */
       $obj = new User();
       $obj->bla = "blub"; // ...
       return $obj;
    }

}

class User {
   private $username, $password, $email, $address_info, $rights, $blub, $bla;

   public final function __get($key) {
      if ( isset($this->username) ) return $key; else return false;
   }

   public final function __set($key, $value) {
      // prevent overwriting existing properties..
      if ( $this->$key === NULL ) $this->$key = $value;
   }
}

Trenne Datenbank anfragen von deiner Register Klasse, du brauchst auch keine Registration-Instanz in deiner User-Klasse, du brauchst nur ein Objekt das den User Identifiziert. das wäre dann die Klasse User.

Ich denke das genügt dir schon als Denkanstoß.

Chriz · 30.01.2011, 15:58

Musst du wissen.

Baltasar · 30.01.2011, 17:12

Erstmal danke für eure Hilfe. Ich hab noch ne Menge zu erlernen. Leider geht ja nicht alles auf einmal und so taste ich mich aktuell ran

Ich habe außerdem den Fehler gefunden:

Ich habe einen neuen User erstellt. Dabei wurde eine neue Instanz Registration erstellt. Im Konstruktor dieser Klasse wurde die controlInput aufgerufen und kontrolliert. War der Username nicht vorhanden, wurde der User stellt und der Name eingetragen. War er vorhanden, war er eben schon da und nichts weiter ist passiert. So oder so: Wenn der Konstruktor durchgelaufen war, gab es auf jeden Fall einen User mit dem eingegebenen Namen.

Anschließend habe ich dann nochmal für weitere Ausgaben und verzweigungen die controlInput aufgerufen und die konnte nun ja nur noch das Ergebnis zurückliefern, dass der Name vorhanden ist. Wieder mal ein klassischer Denkfehler^^

Themen-Optionen
Druckbare Version zeigen Jemanden per E-Mail auf dieses Thema hinweisen
Thema bewerten
Thema bewerten:

Ähnliche Themen
Thema	Autor	Forum	Antworten	Letzter Beitrag
Umlaut Problem	nokieone	PHP Tipps 2009	7	16.10.2009 19:30
Eure Tipps/Hinweise/Empfehlungen zum Thema Modellierung und OOP	ChillSn	Off-Topic Diskussionen	1	30.09.2009 15:19
Problem bei der deklaration einer Datumsvariablen OOP PHP5	litterauspirna	PHP Tipps 2009	12	26.06.2009 04:10
Problem beim Captcha Aufruf in einem Formular, nur das Bild wird angezeigt	litterauspirna	PHP Tipps 2009	19	11.05.2009 15:25
[Erledigt] OOP $row[] mehrfach!	xe0n	PHP Tipps 2009	8	06.05.2009 23:11
Problem mit Basedir in PhP	cmuch05	Datenbanken	9	08.01.2009 14:17
[Erledigt] Problem: PHP Selbe Datei mehr als 1 Mal	soulan	PHP Tipps 2008	13	28.11.2008 22:51
Problem Upload mehrere Dateien und schreiben in DB	Lapje	PHP Tipps 2008	1	22.10.2008 13:12
utf8 Problem (Daten aus DB auslesen)	go1denboy	PHP Tipps 2008	1	23.09.2008 12:19
Problem mit swfobjects	R4v3r	JavaScript, Ajax und mehr	8	21.09.2008 14:17
[Erledigt] Galerie Problem...	coraplanet	PHP Tipps 2008	4	06.06.2008 13:42
datensätze defekt oder problem mit dem einlesen?	Ministry	Datenbanken	4	06.07.2006 18:42
problem!!!		PHP Tipps 2006	6	08.02.2006 11:06
Smarty und PHP-Skript Problem		PHP Tipps 2004-2	2	03.12.2004 22:27
Problem mit alter JavaScript-Funktion	woods	PHP Tipps 2004	1	13.08.2004 13:34

30.01.2011, 12:44
Baltasar Erfahrener Benutzer Registriert seit: 26.10.2003 Beiträge: 592	[Erledigt] OOP: Problem mit Funktionsauswertung Mahlzeit, ich programmiere schon was länger in PHP, aber meist nur kleinere Sachen und die ich habe stets imperativ gelöst. Nun möchte ich aber mein Wissen erweitern und auch in PHP in die Welt der OOP eindringen. Daher programmiere ich der Zeit ein User-System mit Registratur und Login und allem drum und dran. Ich sage gleich vorweg: Ich weiß, dass es etliche Frameworks und Scripte gibt, die das 1000x besser gelöst haben als ich es gerade versuche. Ich will nicht das Rad neu erfinden. Ein Produktiveinsatz ist nicht geplant. Es geht nur darum, dass ich so am besten Lerne. Auf jeden Fall ist der Ablauf wie folgt: Man registriert sich über ein übliches Formular (Username, Passwort, Email). Anschließend wird der Username überprüft. Wenn nicht: Datenbankeintrag und Aktivationsmail. Wenn doch: Nichts davon und ein entsprechender Hinweis. Das Problem ist nun, dass ich immer den Hinweis bekomme, dass der Name belegt ist, auch wenn es nachweislich nicht der Fall ist. Und Trotz der Fehlermeldung laufen alle Prozesse im Hintergrund (Datenbankeintragung etc.) fallunterscheidend richtig. Es geht mir nur um die Meldung. Der Prozess geht von der register.php aus. Dort gibt es folgende If-Kontrolle: PHP-Code: `if (isset($_POST['username'])) { $user = new User($_POST['username'], sha1($_POST['password']), $_POST['email']); if ($user->register() == true) { echo('Registration erfolgreich. Ihnen wurde ein Aktivierungslink an die angegebene Adresse geschickt. '); } else { echo('Username wird leider schon verwendet. '); } }` Die Klasse User liegt in der class.User.php und der entscheidende Teil sieht wie folgt aus. Dort wird ein neues Objekt vom Typ registration erzeugt und der User wird als Parameter übegeben. PHP-Code: `public function register() { $this->registration = new Registration($this); if ($this->registration->controlInput() == false) {return false;} return true; }` Die Klasse Registrations liegt in der class.Registration.php. Dort sieht der entscheidende Teil (die Methode controlInput() ) so aus: PHP-Code: public function controlInput() { $findExistingUser = "SELECT `username` FROM `user` WHERE `username` = '" . $this->user->getUsername() . "'"; $executeSql = mysql_query($findExistingUser) or die(mysql_error()); while($row = mysql_fetch_object($executeSql)) { return false; } return true; } Der Konstruktor der Registration sieht wie folgt aus (und liegt natürlich ebenfalls in der Klasse) PHP-Code: `public function __construct($pUser) { $this->user = $pUser; if ($this->controlInput() == true) { $this->setSQL(); $this->activationMail(); } }` Um das Problem nochmal zusammenzufassen: Ich erzeuge bei der Registration ein Objekt "user". Es wird die Methode "register()" aufgerufen, die ihrerseits ein Objekt vom Typ "Registration" erzeugt und die Kontrolle (ob der Username schon vergeben ist) mittels "controlInput()" vornimmt. Der Kontruktur fragt das Ergebnis ab. Nur im Falle von "true" wird mittels "setSQL()" und "activationMail()" der User erstellt. Die Idee war, dass nun im Falle von "false", dieses "false" durch die Methoden hinweg abgefragt und weitergereicht wird. Aber anscheinend ist da irgendwo der Wurm drin. Ich weiß, dass das nun eine Menge Infos waren und hoffen, dass jemand da mal drüber gucken kann. Ich habe mir Mühe gegeben wie wirklich entscheidenden Code-Fragmente rauszusuchen. Beste Grüße und schon mal vielen Dank, Der Baltasar


Mister Ad PHP Code Flüsterer Registriert seit: 21.08.2005 Beiträge: 4682 PHP-Kenntnisse: Fortgeschritten

30.01.2011, 13:00
curryworld Erfahrener Benutzer Registriert seit: 17.01.2011 Beiträge: 171 PHP-Kenntnisse: Anfänger	gibt dir denn $this->user->getUsername() den richtigen Usernamen zurück? und warum testest du mit einer Schleife und nicht mysql_num_rows?

30.01.2011, 13:16
Chriz Moderator Registriert seit: 11.05.2008 Beiträge: 6.267	Hallo, ob ein Benutzername existiert oder nicht prüfst du mit SELECT COUNT(*) FROM user WHERE `username` = '<USERNAME>'. Das Ergebnis, das immer existiert, holst du mit mysql_result() ab. Eine Schleife brauchst du nicht. Wenn es 0 ist, ist der Benutzername noch frei. Übrigens ist dein Script anfällig für SQL-Injections, benutz mysql_real_escape_string() um das zu verhindern. Deine Kopplung von Datenbezug, Registrierung und "controlInput" finde ich auch nicht sehr gut, trenn doch die Zuständigkeiten etwas besser. __________________ "Nuschel ich?" - "Was?"

30.01.2011, 14:00
Baltasar Erfahrener Benutzer Registriert seit: 26.10.2003 Beiträge: 592	Ich weiß, dass das Script (noch) anfällig für Injections ist. Ich wollte erstmal das Gerüst ans laufen kriegen. Danke aber für den Hinweis Den Wiki-Artikel werde ich mir nachher auch noch zu Gemüte führen. Danke auch dafür. Nun habe ich die "controlInput()" wie folgt überarbeitet. Am Ergebnis ändert sich trotzdem nichts. PHP-Code: public function controlInput() { $findExistingUser = "SELECT COUNT(*) FROM `user` WHERE `username` = '" . $this->user->getUsername() . "'"; $executeSql = mysql_query($findExistingUser) or die(mysql_error()); if (mysql_result($executeSql,0,0) != 0){ return false; } return true; } "$this->user->getUsername()" habe ich gerade mal testweise ausgeben lassen und es liefert auch den richtigen Usernamen zurück. Evtl. hat noch jemand ne Idee?

30.01.2011, 15:49
Baltasar Erfahrener Benutzer Registriert seit: 26.10.2003 Beiträge: 592	Eigentlich heißt das Ding controlInput(), da später auch die Email-Adresse analog zum Usernamen geprüft werden soll. In einem Abwasch sozusagen. Ich hänge halt aktuell nur schon beim Usernamen. var_dump() bringt mich gerade auch nicht weiter, werde aber ab jetzt einsetzen. Naja...mal gucken. Edit: Zur Überarbeitung mit Injections: Methoden, die schon fertig sind (setSql zu, Beispiel) sind bereits mit "mysql_real_escape_string" versehen. Ich mache das immer Abschnittsweise.

30.01.2011, 15:57
tr0y Erfahrener Benutzer Registriert seit: 26.07.2010 Beiträge: 4.874 PHP-Kenntnisse: Fortgeschritten	Dein Klassen-Design ist ein wenig ... naja Datenbank: PHP-Code: class DatabaseAction_RegisterUser extends DatabaseAction { public function Register(User $user) { $result = self::$sqlHandle->Query(/* SQL Query * /); /* error processing / return ...; } public function isRegistred(User $user) { $result = self::$sqlHandle->Query(/ SQL Query /); / error processing & return von true oder false / } public function isRegistrationAllowed() { / error processing & return von true oder false / $result = self::$sqlHandle->Query(/ SQL Query /); } } class DatabaseAction_GetUser extends DatabaseAction { public function getUserdataByID($id) { ... } public function getUserdataByEmail($id, limit=false) { ... } public function getUserdataByState($id, limit=false) { ... } / ect.. pp / } abstract class DatabaseAction { protected static $sqlHandle; public static function CreateHandle($server,$user,$pass,$database) { self::$sqlHandle = new mysqli(...); / fehler behandlung und return / } } User Related Class: PHP-Code: class UserFactory { public function Registrate($arrayOfUserdata) { $view = new DatabaseAction_RegisterUser(); $user = $this->createUserObjectByArray($arrayOfUserdata); if ( $view->isRegistred($user) ) { / ... / return false; } else { / ... / return $this->createUserObjectAfterRegister($id); } } public function Generate($userid) { return $this->createUserObjectByUserID($userid); } public function Delete(User $user) { / ... / } // --- protected function createUserObjectByArray($array) { / ... / $obj = new User(); $obj->blub = "bla"; // ... } protected function createUserObjectByUserID($id) { / ... / $obj = new User(); $obj->bla = "blub"; // ... return $obj; } } class User { private $username, $password, $email, $address_info, $rights, $blub, $bla; public final function __get($key) { if ( isset($this->username) ) return $key; else return false; } public final function __set($key, $value) { // prevent overwriting existing properties.. if ( $this->$key === NULL ) $this->$key = $value; } } Trenne Datenbank anfragen von deiner Register Klasse, du brauchst auch keine Registration-Instanz in deiner User-Klasse, du brauchst nur ein Objekt das den User Identifiziert. das wäre dann die Klasse User. Ich denke das genügt dir schon als Denkanstoß. __________________ Lasse mir ohne Anwendung von Gewalt Dinge schenken, Amazon weiß darüber bald mehr. Geändert von tr0y (30.01.2011 um 16:38 Uhr).*

30.01.2011, 15:58
Chriz Moderator Registriert seit: 11.05.2008 Beiträge: 6.267	Musst du wissen. __________________ "Nuschel ich?" - "Was?"

30.01.2011, 17:12
Baltasar Erfahrener Benutzer Registriert seit: 26.10.2003 Beiträge: 592	Erstmal danke für eure Hilfe. Ich hab noch ne Menge zu erlernen. Leider geht ja nicht alles auf einmal und so taste ich mich aktuell ran Ich habe außerdem den Fehler gefunden: Ich habe einen neuen User erstellt. Dabei wurde eine neue Instanz Registration erstellt. Im Konstruktor dieser Klasse wurde die controlInput aufgerufen und kontrolliert. War der Username nicht vorhanden, wurde der User stellt und der Name eingetragen. War er vorhanden, war er eben schon da und nichts weiter ist passiert. So oder so: Wenn der Konstruktor durchgelaufen war, gab es auf jeden Fall einen User mit dem eingegebenen Namen. Anschließend habe ich dann nochmal für weitere Ausgaben und verzweigungen die controlInput aufgerufen und die konnte nun ja nur noch das Ergebnis zurückliefern, dass der Name vorhanden ist. Wieder mal ein klassischer Denkfehler^^