[PHP] Hilfe bei Prepared Statements

[Scream]

Ich möchte ein vorhandenes Script etwas umschreiben.
Leider bekomme ich eine Fehlermeldung und ich weiß nicht wirklich was ich falsch mache. Kommt vermutlich auch daher dass ich mit OOP bisher so gut wie nichts gemacht habe.
Hier mal mein Code:

config.php
class connect {

public function mysql_verbinden()
{
$user = "testuser";
$password = "testpasswort";

$verbindung = new PDO("mysql:host=localhost;dbname=test", $user, $password);
}
}

funktionen.php
require ('config/config.php');

function holeBenutzerId($benutzername)
{
$test = new connect();
$test->mysql_verbinden();

$sql = "SELECT benutzerid FROM benutzer WHERE benutzername = :benutzername";

$stmt = $verbindung->prepare($sql);


$stmt->bindParam( ':benutzername', $benutzername);




$result = $stmt->execute();
$data = $result->fetch(PDO::FETCH_OBJ);

echo $data->benutzerid;

$result = null;
$stmt = null;
$verbindung = null;
}

Fatal error: Call to a member function prepare() on a non-object in funktionen.php

[Berni]

In der Connect-Klasse gilt die Variable $verbindung ja nur innerhalb der Funktion. Du musst nach "class connect{" noch mit "public $verbindung;" diese Variable als Klassenvariable definieren. Des Weiteren musst du unten dann auch statt "$stmt = $verbindung->prepare($sql);" "$stmt = $test->verbindung->prepare($sql);" schreiben (das "$verbindung = null;" wäre übrigens analog abzuändern)!

Im Übrigen halte ich es nicht für ideal, dass du den connect-Befehl in so ne Funktion reinbaust. Ich gehe ja davon aus, dass du noch mehr solch ähnliche Funktionen basteln wirst und da ist es wesentlich besser, wenn du einmal im ganzen Programm eine Verbindung aufbaust und dann immer auf dieser operierst anstatt in jeder Funktion eine neue Verbindung zu öffnen. Die Verbindung kannst du ja den Funktionen dann entweder übergeben oder mit "global $verbindung" übernehmen.

[Scream]

stimm damit hast du recht
ich hab auch zuerst einmal nur rumprobiert ob ich das überhaupt kann ;)

versuche heute abend den code anzupassen und stelle ihn nochmal hier rein
hoffentlich schaffe ich das auch, denn wie gesagt arbeite ich das erste mal so richtig mit klassen

ist es sinnvoll die connect klasse zu extenden?

[darph]

ist es sinnvoll die connect klasse zu extenden?Eher nicht, weil du dann jede Menge parallele Verbindungen zu DB aufmachst. Da dein Script aber eh sequentiell abläuft und immer nur eine Abfrage gleichzeitig läuft, brauchst du nur eine einzige Verbindung. Hier würde sich ein Singleton anbieten, denn alle deine Klassen, die Statements bereitstellen, teilen sich ja die eine Ressource.

Bei der Gelegenheit ein paar Ratschläge.
Programmiere durchgehend auf Englisch. Deutsch und Englisch zu mixen führt nur zu Verwirrung.
Und schreibe Abkürzungen aus. Wenn du programmieren kannst, kannst du eh schnell tippen, verlierst durch die paar Zeichen also keine Zeit. Dafür gewinnst du den vorteil "sprechender" Variablen. Du siehst also auf den ersten Blick, was die Variable/Methode/Funktion tut und mußt nicht erst überlegen, was die Abkürzung doch gleich bedeutet hat. Du sparst die so etliche Zeilen Dokumentation.
Klassen werden, wenn sie instantiiert werden, zu Objekten. Objekte sind Dinge. Nomen. Methoden sind, wie der Name impliziert, Beschreibungen, wie etwas zu tun sind. Aktionen, die durch oder mit deinem Objekt ausgeführt werden. Verben. Das solltest du trennen. deine Klasse würde ich, wenn, dann "Connection" nennen, nicht connect. Disclaimer, nur für den Herrn Yond: Aber das ist alles nur meine Meinung.

[Scream]

ich habe mal versucht eure verbesserungsvorschläge umzusetzten
bin aber nicht arg weit gekommen *schäm*

jetzt sieht es so aus:

class Connection {

public $connect;


public function Connection() // Konstruktor der automatisch verbindet
{
$user = "testuser";
$password = "testpasswort";

$this->connect = new PDO("mysql:host=localhost;dbname=cdcol", $user, $password);
}



function holeBenutzerId($benutzername)
{
$sql = "SELECT benutzerid FROM benutzer WHERE benutzername = :benutzername";

$stmt = $this->connect->prepare($sql);


$stmt->bindParam( ':benutzername', $benutzername);




$result = $stmt->execute();
$data = $result->fetch(PDO::FETCH_OBJ);

echo $data->benutzerid;

$this->result = null;
$this->stmt = null;
$this->connect = null;
}



jetzt kommt folgender fehler:
Fatal error: Call to a member function fetch() on a non-object in


könnt ihr mir vielleicht nochmal helfen?
kann ich immer noch etwas besser machen?

[Berni]

Ungetestet würde ich mal sagen, dass es so aussehen müsste

public function holeBenutzerId($benutzername)
{
$sql = "SELECT benutzerid FROM benutzer WHERE benutzername = :benutzername";

$stmt = $this->connect->prepare($sql);

$stmt->bindParam( ':benutzername', $benutzername);

$stmt->execute();
$data = $stmt->fetch(PDO::FETCH_OBJ);

echo $data->benutzerid;
}

"$this->connect = null;" ist auf jeden Fall falsch am Ende der Funktion, da du damit ja die Verbindung wieder schließt! Die anderen Variablen musst du eigtl. auch nicht auf null setzen.
Das Problem mit der Fehlermeldung war bei dir, dass du im Grunde "$stmt->execute()->fetch(PDO::FETCH_OBJ);" ausgeführt hast und das geht nicht. Du musst das fetch auf $stmt ausführen.

[Scream]

herzlichen dank :)
hab es auch selber herausgefunden, dass execute() nur einen Booleschen Wert über den Erfolg der Abfrage liefert


habe jetzt aber noch eine frage zu weiteren methoden in der Klasse
bis jetzt sieht es so aus:

class Connection {

public $connect;


public function Connection()
{
$user = "root";
$password = "";

try
{
$this->connect = new PDO("mysql:host=localhost;dbname=cdcol", $user, $password);
}
catch (PDOException $e)
{
echo 'Fehler beim Öffnen der Datenbank: ' . $e->getMessage();
}
}


function holeBenutzerId($benutzername)
{
$sql = "SELECT benutzerid FROM benutzer WHERE benutzername = :benutzername";

$stmt = $this->connect->prepare($sql);
$stmt->bindParam( ':benutzername', $benutzername);
$stmt->execute();
$data = $stmt->fetch();
return $data;
}

function Rights($typ)
{
$benutzerid = new Connection();
$benutzerid->holeBenutzerId($_SESSION['benutzername']);
echo var_dump($benutzerid);

$stmt = $this->connect->prepare($sql);
$stmt->bindParam( ':benutzerid', $benutzerid); // ZEILE DES FEHLERS
$result = $stmt->execute();
if(!$stmt) echo "FEHLER BEI ABFRAGE (rechte): " . mysql_error() . "<br";

$data = $stmt->fetch();
}
}



thoeretisch müsste doch $benutzerid die ID zurückliefern welche in der ersten Methode herausgefunden wurde.
Leider liefert es bei mir folgendes zurück: object(Connection)#3 (1) { ["connect"]=> object(PDO)#4 (0) { } }

Ich denke ich habe in der 2ten Methode bestimmt die erste falsch aufgerufen, denn es kommt folgende Fehlermeldung
Catchable fatal error: Object of class Connection could not be converted to string in...
Was mache ich nun wieder falsch!?

Abgesehen davon kann ich sonst noch etwas verbessern?

[Berni]

$benutzerid->holeBenutzerId($_SESSION['benutzername']);
liefert einen Rückgabewert, den du aber abholen müsstest mit z.B.
$result = $benutzerid->holeBenutzerId($_SESSION['benutzername']);
echo $result->benutzerid

Das weiter unten wundert mich gar nicht, weil ja "$sql" nicht mal definiert ist? Wie soll da ein Prepared Statement gehen?

Generell würd ich keinen Connectbefehl oder Ablauf in deine Funktionen/Klasse schreiben, sondern das ganze dann prozedural außerhalb der Klasse. Also z.B. in ner extra Datei und dann so in der Art
include('/lib/class_connect.php'); //Datei mit der connect-Klasse
$connection = new Connect();
$userid = $connection->get_userid($_SESSION['benutzername']);
$rights = $connection->get_rights($userid);

[Scream]

unten wundert mich gar nicht, weil ja "$sql" nicht mal definiert ist? Wie soll da ein Prepared Statement gehen?


hmm das ist irgendwie beim kopieren verloren gegangen, im code hab ich das natürlich ;)

[Berni]

Ok ;) Auf jeden Fall liegts auch daran, dass du eine falsche Variable zuweist:
$stmt->bindParam( ':benutzerid', $benutzerid);
sollte eher
$stmt->bindParam( ':benutzerid', $result->benutzerid);
sein (vgl. dazu auch meinen vorigen Post). Denn du musst dort als Variable einen String übergeben, bei dir wurde aber die Verbindung übergeben ("$benutzerid=new Connection()")!

[The_Invisible]

wenn du von OOP noch nicht so nen plan hast würde ich dir http://www.php.net/manual/de/language.oop5.php empfehlen.

hier bekommst du einen guten überblick was alles möglich ist und was nicht. das meiste wirst du zwar nicht oder kaum brauchen, aber es ist nicht schlecht wenn man das wenigstens einmal "überflogen" hat.

da pdo eh erst ab version 5.0 von php funktioniert gehe ich mal stark davon aus das du eh hauptsächlich mit dieser version von php programmierst. php4 biete in sachen OOP sehr viel weniger.

mfg

[Scream]

@The_Invisible
danke das werd ich mir gut durchlesen

noch ne frage zu fetch
$data = $stmt->fetch(PDO::FETCH_OBJ);
return $data->benutzerid;
Es gibt ja sehr viele vordefinierte Konstanten. Ist es sinnvoll wie ich es mache (siehe Code)
Oder gäbs auch nen besseren weg?

[Scream]

sorry für den doppelpost aber ich glaub ich habs endlich geschafft so wie ichs wollte :)

vielleicht schaut ihr nochmal kurz drüber ;)

startseite.php

require("funktionen.php");
$pagename = 'startseite';
$ANGEMELDET = angemeldet();
$RECHTE = new Connection();
$userid = $RECHTE->holeBenutzerId($_SESSION['benutzername']);
$rights = $RECHTE->Rights($pagename,$userid);
$inhalt = $RECHTE->Inhalt($pagename);


funktionen.php
class Connection {

public $connect;


public function Connection()
{
$user = "root";
$password = "";


try
{
$this->connect = new PDO("mysql:host=localhost;dbname=cdcol", $user, $password);
}
catch (PDOException $e)
{
echo 'Fehler beim Öffnen der Datenbank: ' . $e->getMessage();
}
}


public function holeBenutzerId($benutzername)
{
$sql = "SELECT benutzerid FROM benutzer WHERE benutzername = :benutzername";
$stmt = $this->connect->prepare($sql);
$stmt->bindParam( ':benutzername', $benutzername);
$stmt->execute();
if(!$stmt) echo "FEHLER BEI ABFRAGE (benutzerid)<br";
$data = $stmt->fetch(PDO::FETCH_OBJ);
return $data->benutzerid;

}


public function Rights($typ,$userid)
{
$sql = "SELECT $typ FROM benutzerrechte WHERE benutzerid = :benutzerid";
$stmt = $this->connect->prepare($sql);
$stmt->bindParam( ':benutzerid', $userid);
$result = $stmt->execute();
if(!$result) echo "FEHLER BEI ABFRAGE (rechte)<br";
$data = $stmt->fetch(PDO::FETCH_OBJ);
return $data->$typ;
}


public function Inhalt($typ)
{
$sql = "SELECT $typ FROM sonstiges";
$stmt = $this->connect->prepare($sql);
$result = $stmt->execute();
if(!$result) nachrichtenbox("fehler","DB-Fehler bei ".$typ);
$data = $stmt->fetch(PDO::FETCH_OBJ);
return $data->$typ;
}

}

[The_Invisible]

den Wert für $pagename könntest du direkt dem konstruktor übergeben und in der klasse als "private var" speichern, somit entfallen damit schon mal die jeweiligen parameter für die methoden Rights() und Inhalt().

mit der $userid könntest du es genauso machen, also das nach dem aufruf der methode holeBenutzerId() die $userid in der klasse gespeichert wird und der parameter $userid für die methode Rights() dann nur mehr optional ist bzw er standardmäßig die Rechte der zuletzt gefundenen $userid zurückgibt.

mfg

[Scream]

ok ich habe meinen ansatz nochmal wie darph vorgeschlagen hat überarbeitet

hoffentlich ist das jetzt die letzte frage ;)

<?php
class Connection {

// Speichert die Instanz der Klasse
private static $instance;

// Ein private Konstruktor; verhindert die direkte Erzeugung des Objektes
private function __construct()
{
$dbname = "cdcol";
$user = "root";
$password = "";

try
{
$this->instance = new PDO("mysql:host=localhost; $dbname", $user, $password);
}
catch (PDOException $e)
{
echo 'Fehler beim Öffnen der Datenbank: ' . $e->getMessage();
}
}

// Die Singleton Funktion
public static function getInstance()
{
if (!isset(self::$instance)) {
self::$instance = new Connection();
}

return self::$instance;
}

// Halte Benutzer vom Klonen der Instanz ab
public function __clone()
{
trigger_error('Klonen ist nicht erlaubt.', E_USER_ERROR);
}


public function holeBenutzerId($benutzername)
{
$sql = "SELECT benutzerid FROM benutzer WHERE benutzername = :benutzername";
$stmt = $this->instance->prepare($sql);
$stmt->bindParam( ':benutzername', $benutzername);
$result = $stmt->execute();
if(!$result) echo "FEHLER BEI ABFRAGE (benutzerid)<br";
$data = $stmt->fetch(PDO::FETCH_OBJ);
return $data->benutzerid;
}

public function Rights($typ,$userid)
{
$sql = "SELECT $typ FROM benutzerrechte WHERE benutzerid = :benutzerid";
$stmt = $this->instance->prepare($sql);
$stmt->bindParam( ':benutzerid', $userid);
$result = $stmt->execute();
if(!$result) echo "FEHLER BEI ABFRAGE (rechte)<br";
$data = $stmt->fetch(PDO::FETCH_OBJ);
return $data->$typ;
}
}


Nun will ich dass die Methode holeBenutzerId und Rights auf meine Connection zugreifen können, aber irgendwie funktioniert das nicht weil $instance ja static ist.

Meine index.php beinhaltet folgendes, muss ich daran auch noch was ändern oder nur in den Methoden?
$RECHTE = Connection::getInstance();
$userid = $RECHTE->holeBenutzerId($_SESSION['benutzername']);
$rights = $RECHTE->Rights($pagename,$userid);

[Berni]

Also so ganz verstehe ich das Problem nicht, der Code wird bei mir so problemlos ausgeführt (bis auf das Meckern bzgl. der nicht vorhandenen MySQL-Tabellen).

Das ganze Rumgebastle bzgl. einer statischen Klasse mit Singleton hätte ich nicht gemacht weil ich keinen Vorteil sehe; der Sicherheitsgewinn ist nicht vorhanden und wenn man weiß was man tut erstellt man soundso nicht mehrere Instanzen einer DB-Klasse weil das nur einmal in einer configuration.php oder global.php gemacht wird. Aber gut, jetzt hast dus ja eh schon ;)

Ich würde "$RECHTE" in "$db" umbenennen, weil das ja die richtige Bezeichnung dafür ist. Außerdem folgst du darphs Empfehlung einer durchgehenden englischen Bezeichnung der Variablen nicht ("holeBenutzerId"). Da hat er durchaus Recht. Besonders so ein Mix aus englisch und deutsch ist nicht wirklich optimal vor Allem wenn dann noch Variablen in beiden Sprachen vorkommen ("rights" und "RECHTE")...

Des Weiteren würde ich in die Klasse nur eine generische Funktion namens "query_first" reinmachen, der man die Abfrage und ein Array mit den Prepared-Statements-Variablen übergeben kann und die ein Ergebnis-Array zurückgibt. Dann kann man nämlich schön auch weitere Abfragen einfach gestalten und braucht nicht X redundante Funktionen. Zudem ist wohl auch eine Funktion "query" nötig, bei der man dann selber im normalen Programmcode das "fetch" in einer while-Schleife übernehmen kann.

[Scream]

ja das mit den benennung wollte ich ändern sobald alles funktioniert

wenn ich nach "$result = $stmt->execute();" ein "echo var_dump($result);" mache bekomme ich false zurück
also irgendwie wird das nicht ausgeführ und ich denke das liegt an der Zeile:

$stmt = $this->instance->prepare($sql);

[Berni]

$_SESSION['benutzername'] ist ja auch gar nicht gefüllt, so dass er wohl gar nichts finden dürfte.

[Scream]

klar ist das gefüllt ;)
das lasse ich vorher füllen, hab ich nicht im code hier gepostet
und vorher gings ja auch perfekt bevor ich das singleton konstruiert habe

[Scream]

so ich habs hinbekommen danke an alle!
werde mich evtl nochmal melden am WE um weitere Vorschläge entgegenzunehmen :)