[PHP] Klassenstruktur für Adressdatenbank

[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