[PHP] Klassenstruktur für Adressdatenbank

[Valle]

Zitat von Luckie:

Klingt gut. auch wenn ich es selber bastele, werde ich mir das angucken.

Andersrum wäre schlauer, erst gucken, dann machen. Aber sonst gute Idee.

Ich hoffe es macht dir so viel Spaß wie mir, denn ich musste mich echt zurückhalten nicht schon wieder sowas anzufangen.

Liebe Grüße,
Valentin

[Luckie]

Könnte ich damit glücklich werden:

Code:

<?php

class DB_Exception extends Exception {}

class Contact extends DB_Exception{

   private $connection;
   private $resultset;
   public $id = array();
   public $name = array();
   public $vorname = array();
   
   function __construct() {
   
      include_once("config.php");
      
      $this->connection = mysql_connect($dbserver, $user, $password);
      if ($this->connection == false)
      {      
         die(mysql_error());
      }
      $db = mysql_select_db("usr_l3s11195_1");
      if ($db == false)
      {      
         die(mysql_error());
      }
   }
   
   function __destruct()  {
      mysql_free_result($this->resultset);
      mysql_close($this->connection);
   }
   
   public function getByName() {         
      $query = "SELECT
               a.*, k.name as k_name
               FROM adressen_data a
               LEFT OUTER JOIN adressen_kat k on k.id = a.kategorie_id
               WHERE a.name LIKE '%".$this->name[0]."%'
               ORDER BY k.id, a.name, a.vorname, a.gesch_firma";
      $this->resultset = mysql_query($query);
      if (!$this->resultset) {
         throw new DB_Exception(@mysql_error());
      }      
      
      $this->clearFields();
      $this->fillFields();
   }
   
      public function getBySurname() {         
      $query = "SELECT
               a.*, k.name as k_name
               FROM adressen_data a
               LEFT OUTER JOIN adressen_kat k on k.id = a.kategorie_id
               WHERE a.vorname LIKE '%".$this->vorname[0]."%'
               ORDER BY k.id, a.name, a.vorname, a.gesch_firma";
      $this->resultset = mysql_query($query);
      if (!$this->resultset) {
         throw new DB_Exception(@mysql_error());
      }      
      
      $this->clearFields();
      $this->fillFields();
   }
   
   public function getById() {         
      $query = "SELECT
               a.*, k.name as k_name
               FROM adressen_data a
               LEFT OUTER JOIN adressen_kat k on k.id = a.kategorie_id
               WHERE a.id = '".$this->id[0]."'
               ORDER BY k.id, a.name, a.vorname, a.gesch_firma";      
      $this->resultset = mysql_query($query);
      if (!$this->resultset) {
         throw new DB_Exception(@mysql_error());
      }      
      
      $this->clearFields();
      $this->fillFields();
   }
   
   public function save() {
   
   }
   
   public function edit() {
   
   }
   
   private function fillFields() {
      while ($row = mysql_fetch_object($this->resultset)) {
         $this->id[] = $row->id;
         $this->name[] = $row->name;
         $this->vorname[] = $row->vorname;            
      }      
   }
   
   private function clearFields() {
      unset($this->id);
      unset($this->name);
      unset($this->vorname);
   }
}

?>

Code:

<?php

include_once("Contact.php");

   class View {
      private $contact;
      
      public function getContactByName($filterStr) {
         try {
            $this->contact = new Contact();
            $this->contact->name[0] = $filterStr;         
            $this->contact->getByName();
            $this->htmlOutPut();
         }
         catch(DB_Exception $e) {
            echo $e->getMessage();
         }
      }   
      
      public function getContactBySurname($filterStr) {
         try {
            $this->contact = new Contact();
            $this->contact->vorname[0] = $filterStr;         
            $this->contact->getBySurname();
            $this->htmlOutPut();
         }
         catch(DB_Exception $e) {
            echo $e->getMessage();
         }
      }

      public function getContactById($Id) {
         try {
            $this->contact = new Contact();
            $this->contact->id[0] = $Id;         
            $this->contact->getById();
            $this->htmlOutPut();
         }
         catch(DB_Exception $e) {
            echo $e->getMessage();
         }
      }
      
      private function htmlOutPut() {
         for($i=0; $i < count($this->contact->id); $i++) {
            if (count($this->contact->id) > 1) {
               echo "ID: ".$this->makeLink("index.php?action=show&id=".$this->contact->id[$i], $this->contact->id[$i])."<br>\n";
               echo "Name: ".$this->contact->name[$i]."<br>\n";
               echo "Vorame: ".$this->contact->vorname[$i]."<br>\n";
               echo "<br>\n";
            } else {
               echo "Name: ".$this->contact->name[$i]."<br>\n";
               echo "Vorame: ".$this->contact->vorname[$i]."<br>\n";
               echo "<br>\n";
               echo $this->makeLink("index.php", "Bearbeiten")." | ".$this->makeLink("index.php", "Löschen")."\n";
            }
         }   
         unset($contact);
      }
      
      private function makeLink($href, $caption) {
         return "<a href=\"".$href."\">".$caption."</a>";
      }
      
   }
?>

Code:

<html>
   <head>
      <meta http-equiv="Content-Type" content="text/html; charset=ISO-8859-1">
   </head>
   <body>
      <form action="index.php" method="GET">      
         <input type="hidden" name="action" value="search">         
         <select name="field" size="1">                        
            <option value="name">Name</option>
            <option value="vorname">Vorname</option>
         </select>      
         <input name="filterStr" type="text"></input>
         <input type="submit" value="Suchen"></input>
      </form>   
      
      <?php
         include_once("View.php");
         
         $action = $_GET['action'];
         $id = $_GET['id'];
         $filterStr = $_GET['filterStr'];
         $field = $_GET['field'];
         
         $view = new View();
         
         switch ($action) {            
            case "search":
               switch ($field) {
                  case "name":
                     $view->getContactByName($filterStr);
                     break;
                  case "vorname":
                     $view->getContactBySurname($filterStr);
                     break;
               }
               break;
            case "show":
               $view->getContactById($id);
               break;
            default:
               echo "<a href=\"index.php\"n>Neuer Kontakt</a>\n";
               break;
         }
      ?>
   </body>
</html>

Es kommen noch ein paar Suchabfragen dazu und die Datenbank hat so um die 20 Felder.

Könnte man auch noch was verbessern oder vereinfachen? Oder könnte ich irgendwo Probleme bekommen?

[Valle]

• class Contact extends DB_Exception? Ich denke das war nicht beabsichtigt, oder?
• Die Kontakte-Klasse sollte die Verbindung zu MySQL nicht selbst aufbauen.
• Man schließt die MySQL-Verbindung auch nicht im Destruktor dieser Klasse, sondern am Ende des Programms.
• Es empfiehlt sich auch eine abstrakte Bibliothek zu verwenden (PDO)
• Damit mindert man auch das Risiko an SQL-Injections, die du hier auch eingebaut hast; Parameter verwenden!
• Dein Exception-Handling ist etwas ungünstig. Mit einer richtigen Bibliothek geht das schlauer (ich habe einen Wrapper geschrieben, der automatisch eine Exception wirft, wenn ein Query fehlschlägt) Normalerweise werden diese an der richtigen Stelle erzeugt und bis zur obersten Instanz, einem Exception-Handler, durchgereicht. Hier kann man fein auf eine DEBUG-Variable prüfen und ggf. eine Meldung ausgeben oder gar eine Mail schicken.
• Statt all die Felder zu kopieren, solltest du Getter und Setter verwenden.
• Dein MVC ist nicht wirklich eins. Dein View übernimmt Exception-Handling und weiß zu viel vom Model. Einen Controller hast du nicht wirklich, bzw. mischst ihn mit dem eigentlichen View (HTML-Code).
• Wenn du glücklich werden willst, verwende eine Template-Engine. Die kann man auch leicht selbst machen
• Als kleiner Tipp: In PHP-Dateien ohne HTML-Code muss man die PHP-Klammer nicht schließen. Nutze das aus, denn damit spart man sich die ein oder andere Leerzeile im Quelltext, welche später (beim Setzen von Cookies und anderen Headers) Kopfschmerzen bereitet.
• Als Anregung könnte ich den Zend Framework Coding Style Guide (o.ä.) empfehlen!

Du machst das schon ganz gut, denn ich habe schon viel schlimmeres gesehen. Man sieht dass du eine eigene Vorstellung hast, von dem was du tust. Allerdings fehlt hier noch einiges an Praxis-Erfahrung, bzw. do's und don'ts. Für meinen Geschmack ist das auch für ein kleines Privatprojekt noch nicht abstrakt genug. Mach deine Klassen kleiner und dümmer, dafür aber mehr davon.

Btw, haben wir jetzt nicht auch [php]?

<?php $answer = 42; Liebe Grüße,
Valentin

[Luckie]

Zitat von Valle:

class Contact extends DB_Exception? Ich denke das war nicht beabsichtigt, oder?

Eigentlich schon. Oder wie kann ich sonst eigene, spezifische Exceptions werfen?

Zitat:

Die Kontakte-Klasse sollte die Verbindung zu MySQL nicht selbst aufbauen.

Na ja, Ich wollte es erst nur mit einer Klasse mache, deswegen der Verbindungsaufbau an dieser Stelle. Wo wäre es denn günstiger?

Zitat:

Man schließt die MySQL-Verbindung auch nicht im Destruktor dieser Klasse, sondern am Ende des Programms.

Hm. Es ist also ungünstig jedes mal wieder eine Verbindung aufzubauen und zu trennen?

Zitat:

Es empfiehlt sich auch eine abstrakte Bibliothek zu verwenden (PDO)

Habe ich mit Absicht vermieden.

Zitat:

Statt all die Felder zu kopieren, solltest du Getter und Setter verwenden.

Ich war erst mal zu faul die Getter und Setter zu schreiben. Leider hab eich keine IDE welche sie automatisch aus den privaten Feldern erzeugt.

Zitat:

Dein MVC ist nicht wirklich eins. Dein View übernimmt Exception-Handling und weiß zu viel vom Model. Einen Controller hast du nicht wirklich, bzw. mischst ihn mit dem eigentlichen View (HTML-Code).

Sollte es auch gar nicht sein.

Zitat:

Mach deine Klassen kleiner und dümmer, dafür aber mehr davon.

Mal schauen, ob ich das hinbekomme.

Danke für die Tipps und Anregungen.

[Valle]

Zitat von Luckie:

Zitat von Valle:

class Contact extends DB_Exception? Ich denke das war nicht beabsichtigt, oder?

Eigentlich schon. Oder wie kann ich sonst eigene, spezifische Exceptions werfen?

Dein Kontakt ist doch keine Exception, sondern eine Tabelle, bzw eine Zeile dieser. Du wirfst ja keinen Kontakt im Fehlerfall, sondern eine extra angelegte Exception. Normalerweise hat man Exceptions wie QueryFailed oder EmptyResult oder sowas. Diese werden von der Vaterklasse des Kontaktes geworfen und jeweils als eigene Klasse implementiert, denn sowas braucht man bei jeder Klasse. Man hat dann für jede Klasse, und wenn sie noch so klein ist, eine eigene Datei. Ich schlage eine DB_Exception Klasse vor (erbt von Exception) und dann die genannten EmptyResult usw. Exceptions, welche von der DB_Exception erben. getById (oder einfach get) sollte auch nicht vom Kontakt selbst implementiert sein, denn eine ID sollte jede Tabelle haben. (m:n zählt nicht)

Zitat von Luckie:

Zitat:

Die Kontakte-Klasse sollte die Verbindung zu MySQL nicht selbst aufbauen.

Na ja, Ich wollte es erst nur mit einer Klasse mache, deswegen der Verbindungsaufbau an dieser Stelle. Wo wäre es denn günstiger?

MySQL-Verbindungen speichere ich immer in einer Registry. Diese ist ein Singleton und stellt eigentlich nur ein assoziatives Array dar. Es gibt also nur eine Registry im Programm.
Man holt sich also die Registry Instanz (siehe Singleton Artikel auf Wikipedia unter PHP), verbindet sich im "Bootloader" mit MySQL und speichert die Connection in diese Registry. Die Db_Table Klasse, von der der Kontakt erben sollte, beschafft sich im Konstruktor diese Verbindung, speichert sie in ein Feld und man arbeitet in der Kontakt-Klasse dann einfach mit $this->db.

Zitat von Luckie:

Zitat:

Man schließt die MySQL-Verbindung auch nicht im Destruktor dieser Klasse, sondern am Ende des Programms.

Hm. Es ist also ungünstig jedes mal wieder eine Verbindung aufzubauen und zu trennen?

Ja natürlich! Was hast du denn gedacht? Für jeden Query Verbindung neu aufbauen und wieder beenden? Eine Verbindung pro Seitenaufruf reicht völlig aus. Es gibt Leute, die gehen dazu über persistente Verbindungen aufzubauen. Diese bauen einmal eine Verbindung auf, welche dann bis zum nächsten Neustart des Webserver aufrecht erhalten bleibt.

Zitat von Luckie:

Zitat:

Es empfiehlt sich auch eine abstrakte Bibliothek zu verwenden (PDO)

Habe ich mit Absicht vermieden.

Schade, warum? Du weißt dass du SQL-Injections riskierst?

Zitat von Luckie:

Zitat:

Statt all die Felder zu kopieren, solltest du Getter und Setter verwenden.

Ich war erst mal zu faul die Getter und Setter zu schreiben. Leider hab eich keine IDE welche sie automatisch aus den privaten Feldern erzeugt.

Das hast du nicht richtig verstanden. Du machst dein Query und holst die Daten des Kontakts. Diese speicherst du als assoziatives Array oder Objekt in ein privates Feld, mit Namen data oder so. Dann machst du zwei Methoden __get und __set. Beispiel:  function __get($name) { return $this->data->$name; }

Zitat von Luckie:

Zitat:

Dein MVC ist nicht wirklich eins. Dein View übernimmt Exception-Handling und weiß zu viel vom Model. Einen Controller hast du nicht wirklich, bzw. mischst ihn mit dem eigentlichen View (HTML-Code).

Sollte es auch gar nicht sein.

Schade, warum?

Zitat von Luckie:

Zitat:

Mach deine Klassen kleiner und dümmer, dafür aber mehr davon.

Mal schauen, ob ich das hinbekomme.

Sicher schaffst du das. Wir helfen dir dabei!

Edit:// Was ich jetzt noch gar nicht bemerkt habe! Du hast alle Methoden nicht-statisch gemacht. Das ist nicht ganz so wie es sein sollte. Stell dir vor, dass die Klasse die Tabelle repräsentiert. Eine Instanz der Klasse repräsentiert eine einzige Zeile dieser Tabelle. Möchtest du also einen speziellen Kontakt, rufst du die statische Methode get($id) der Klasse auf. Diese erzeugt eine Instanz ihrer selbst und gibt sie zurück. Weißt du wie ich das meine?

Liebe Grüße,
Valentin

[Namenloser]

[OT]

Zitat von Valle:

Zitat von Luckie:

Klingt gut. auch wenn ich es selber bastele, werde ich mir das angucken.

Andersrum wäre schlauer, erst gucken, dann machen. Aber sonst gute Idee.

Kommt glaube ich darauf an, was für ein Typ man ist. Manche lernen besser, indem sie Code von anderen studieren, andere lernen besser, indem sie selbst welchen schreiben/versuchen etwas nachzubauen. Ich gehöre selbst zur zweiten Gruppe. Oft wird einem nämlich erst dann klar, warum bestimmte Dinge bei anderen Frameworks/Libs/Was-auch-immer auf eine bestimmte Weise gelöst wurden. Ich hatte auf diese Weise jedenfalls schon einige Aha-Erlebnisse.
[/OT]

[Valle]

Ich hab hier mal ein kleines UML Diagramm skizziert.

Kursive Klassen sind abstrakt, unterstrichene Methoden sind statisch.

Ich garantiere keine Korrektheit oder Vollständigkeit, aber es sollte ungefähr zeigen wie ich mir das vorstelle.

Liebe Grüße,
Valentin

[Luckie]

@Valle: Danke, dass du dir da so deinen Kopf zerbrochen hast. Ich werde es versuchen nach und nach umzusetzen. Ich habe mir mal zwei Sachen rausgepickt als nächsten Milestone:

1. Das mit den Exceptions.
2. Auslagern des Verbindunsgaufbaus.

Für beides bräuchte ich aber zum Angucken und nachbauen etwas Beispielcode.

[Luckie]

So, meine Exceptionklasse sieht jetzt erst mal so aus:

Code:

<?php
   class DBException extends Exception
   {
      public function __construct($message, $code = 0) {
         parent::__construct($message, $code);
      }
   }
   
   class DBEXception_QueryFailed extends DBException{
      public function __construct($message, $code = 0) {
         parent::__construct($message, $code);
      }
   }
   
   class DBEXception_EmptyResult extends DBException{
      public function __construct($message, $code = 0) {
         parent::__construct($message, $code);
      }
   }
?>

Ich habe gelesen, dass man den Konstruktor auf alle Fälle überschreiben und dort den Parent-Konstruktor aufrufen soll. Ich denke mal, so hast du dir das vorgestellt.

[Valle]

Zitat von Luckie:

Ich habe gelesen, dass man den Konstruktor auf alle Fälle überschreiben und dort den Parent-Konstruktor aufrufen soll.

So'n Quatsch. Lass das mal. o0 (Oder mach's von mir aus ein Mal in der Vaterklasse, aber nicht jedes mal)

Btw, PHP-Klammer bei PHP-Code bitte weglassen, dann funktioniert das Highlighting.

PHP-Quellcode:

class DBEXception_QueryFailed extends DBException
{}

Und nochmal btw, du brauchst nach der PHP-Klammer nicht einrücken. Damit sparst du wertvollen Platz nach rechts. Und das Einrücken nach der Klammer ist wirklich sinnlos. (Du rückst in Delphi ja auch nicht nach program oder unit ein. (Oder wie auch immer das da oben heißt^^)

Zitat von Luckie:

Ich denke mal, so hast du dir das vorgestellt.

Perfekt.

Und hier mal eine einfache und ungetestete Implementation der Registry:

PHP-Quellcode:

// Registry.php

final class Registry
{
 
    private static $instance = NULL;
    private $data = array();

    private function __construct() {}
    private function __clone() {}
 
    public static function getInstance() {
 
        if (NULL === self::$instance) {
            self::$instance = new self;
        }
        return self::$instance;
    }

    public function get($id)
    {
        return $this->data[$id];
    }

    public function __get($id)
    {
        return $this->get($id);
    }

    public function set($id, $value)
    {
        $this->data[$id] = $value;
    }

    public function __set($id, $value)
    {
        return $this->set($id, $value);
    }

    // TODO throw exception on missing index
    // TODO unset, __unset, isset, __isset

}
 
// Bootloader.php

abstract class Bootloader
{

    private static function connect_mysql()
    {

        $registry = Registry::getInstance();
        $registry->db = new mysqli('127.0.0.1', '', '', 'mysql');

        if (mysqli_connect_error()) {
            die('Connect Error (' . mysqli_connect_errno() . ') ' . mysqli_connect_error());
        }

        printf("Connected to %s\n", $registry->db->host_info);
    }

    public static function boot()
    {
        register_shutdown_function("Bootloader::shutdown");
        self::connect_mysql();
    }

    public static function shutdown()
    {
        Registry::getInstance()->db->close();
        printf("Connection closed.\n");
    }

}

// index.php

Bootloader::boot();

// IrgendeinModel.php

class Model 
{

    public function __construct()
    {
        $this->db = Registry::getInstance()->get('db');
        echo $this->db->query('SELECT NOW() AS `now`')->fetch_object()->now . "\n";
    }

}

new Model();

Liebe Grüße,
Valentin