PHP PDO Datenbankverbindung in einer Klasse nur einmal aufbauen

[gopeter]

Hallo zusammen,
ich arbeite jetzt zum ersten mal ernsthaft mit OOP in PHP, da bisher Funktionsbibliotheken ausgereicht haben. Da das jetzige Projekt aber ein bisschen größer ist als mein Kleinkram sonst, versuch ichs mal OOP

Dazu habe ich eine Frage:
Ich habe eine Klasse Clients(). Alle Methoden dieser Klasse benötigen Datenbankzugriff, also dachte ich mir, es wäre sinnvoll, den Datenbankaufbau (nutze PDO) gleich in die __constructor Methode zu packen.

1. ist das sinnvoll?

2. so sieht das momentan bei mir aus:

<?php
// include database class
require_once('../../../lib/php/classes/database/database.class.php');

class Client {

	public function __construct() {
        $this->db = Database::get("default");
    }

    public function getClient() {
		$clients = $this->db->select("SELECT * FROM clients ORDER BY name ASC");
		return $clients;
    }
    
    public function newClient($values) {
		if ($this->db->insert("INSERT INTO clients (name,initial,payment,hourly_rate,active) VALUES (?,?,?,?,?)",$values)) {
		} else {
			print "Inserting failed";
		}
	}
	
    public function editClient($values) {
		if ($this->db->update("UPDATE clients SET name=?, initial=?, payment=?, hourly_rate=?, active=? WHERE cid=?",$values)) {
		} else {
			print "Updating failed";
		}
	}
	
	public function __destruct() {
		$this->db = null;
		unset($this->db);
	}	
}
?>

Allerdings ist das auch nicht wirklich sauber, weil ich ja somit die beiden Klasse direkt miteinander verbinde, oder? Habe jedenfalls gelesen, dass man das so nicht machen sollte... hat jemand Ideen hierfür?

 

[Yuuri]

Das hast du wo gelesen? Wenn es dem Komfort dient und du somit nicht dauerhaft Database::get("blabla") aufrufen musst, spricht doch nichts dagegen. Der einzig schlechte Stil bei dir ist, dass du die Variable $db nicht setzt. PHP mag zwar nicht meckern, wirklich schön ist es aber nicht. Zumal somit kein Code-Completion (entsprechender Editor/IDE vorausgesetzt) funktioniert.

Sonst passt der Rest.

Ein Fehler:

$this->db = null;
unset( $this->db );

Entweder umdrehen oder weg lassen. null zu löschen bringt dir null Punkte, da null Effekt. Weg lassen wäre hier aber von Vorteil, denn die Datenbankverbindung willst du ja nicht kappen, der Inhalt des Objekts verschwindet sowieso im Nirvana. Ergo brauchst du nicht.

 

[gopeter]

Ok, danke!

Zu dem Fehler:
Huch, da war ich aktuell noch am rumspielen. Hatte ein Memory Problem und mit verschiedenen Lösungen rumprobiert, daher noch das doppelgemoppel

/edit

Achso...ähm. Aber eine Variable $db sprech ich doch gar nicht an? Wie/Wo würdest du die denn setzen?

 

[Yuuri]

Achso...ähm. Aber eine Variable $db sprech ich doch gar nicht an? Wie/Wo würdest du die denn setzen?

class Client {

    [B]private $db;[/B]

	public function __construct() {
        [B]$this->db[/B] = Database::get("default");
    }

Noch etwas: Ich würde dir auch empfehlen absolute Pfade bei Includes zu verwenden, einfach der Gewissheit halber, dass du immer auf diese eine Datei zugreifst und du weißt, dass sie an diesem Ort liegt. Leg dazu am besten einfach ein paar Konstanten in der index.php oder irgendwo ausgelagert an, die die allgemeinen Pfade beschreiben und von diesen kannst du dann includen. Das spart dir mitunter auch lästiges ../../../../../../../../../../../../../../../../. Ein require_once INC_LIBS_DB.'class.db.php'; sähe schöner aus (Wartbarkeit auch nicht vernachlässigen!) und hätte mehr Semantik, zumal du dir den Pfad nicht merken musst. Du kannst dir natürlich auch eine Include-Klasse basteln, wo du bspw. sagst, du willst eine Lib namens Database einbinden. Wäre der nächste Schritt, vielleicht als Anregung.

Der Rest passt so. Je nach Art und Vorliebe, würde ich die Parameter der Funktionen direkt einsetzen, anstatt Arrays mit Werten zu übergeben. Bei der ersten Methode kannst du dir sicher sein, dass die Variablen (wenn auch nur irgendwie) gesetzt sind. Bei einem Array handtierst du dauerhaft mit isset(), empty() und sonst was herum. Falls es zu viele Parameter werden sollten, kannst du ja auch Standardwerte einfügen im Stile

public function editClient( $ClientId, $ClientIp, $ClientPort = 80, $ClientInfo = '', $ClientData = '') {

Genauso würde ich named Parameter (:<name des platzhalters>) einsetzen, falls die Querys mal abgeändert werden. So kannst du sicher sein, dass sie die richtige Reihenfolge bei der Zuweisung besitzen (welche hierbei wiederum egal wäre) und es ist eindeutig, was wo zugewiesen wird.

Aber wie gesagt: Persönliche Vorliebe und Geschmack sollte nicht unter den Tisch fallen, soll schließlich jeder so programmieren, wie er es für gut befindet.

 

[gopeter]

Super, danke dir vielmals! Das mit den named Parameter habe ich mir auch schon angeschaut und werde ich wohl auch so umsetzen, da die Fragezeichen den Code doch ziemlich unleserlich machen und man zudem das Array nicht versteht, ohne weitere Dateien zu durchforsten.

 

[trialgod]

Was du gelesen hast nennt sich Dependency Injection und ist auch richtig. Und zwar besagt dieses Pattern, dass Klassen sich möglichst nicht untereinander instanziieren, sondern übergeben werden (Um Abhängigkeiten zu vermeiden).

In deinem konkreten Fall, sollte das dann so ausschauen:

class Client
{
    protected $_db;

    public function __construct(Database $db)
    {
        $this->_db = $db;
    }

    ...
}
$db = Database::get('default');
$client = new Client($db)

 

[gopeter]

Super, danke dir! Genau das meinte ich.
Probier ich gleich mal aus

 

[voodoo44]

So würde ich es im übrigen auch umsetzen - allein schon, weil sich damit Unittests viel einfacher realisieren lassen. Mit einer Dependency-Injection lässt sich das viel schöner mocken.