Feedback zu MySQL Klasse

[PHPbeginner]

Hallo Community!

Ich habe mich mal an eine MySQL Klasse herangewagt, da ich mein nächstes Projekt mal etwas objektorientierter angehen möchte.

class DBQuery
{
    //Creates a new MySQL connection
    function DBQuery()
    {
        $this->dbConnection = mysql_connect(VIA_DB_SERVER, VIA_DB_USER, VIA_DB_PASSWORD);
        if (!$this->dbConnection) 
        {
            die("Could not connect: " . mysql_error());
        }
        mysql_select_db(VIA_DB_DATABASE) or die("Could not select database");
    }
    
    //Closes the open MySQL connection
    function close()
    {
        $this->dbConnection = mysql_close();
    }
    
    //Returns a select
    function select($what = '*', $from, $where = null, $limit = null, $orderby = null)
    {
        $queryString = "SELECT " . $what . " FROM " . VIA_DB_PREFIX . $from;
        
        if($where != null)
            $queryString .= " WHERE " . $where;
            
        if($limit != null)
            $queryString .= " LIMIT " . $limit;
            
        if($orderby != null)
            $queryString .= " ORDER BY " . $orderby;
        
        $queryResult = mysql_query($queryString) or die(mysql_error());
        
        $run = 0;
        while($queryResultFetch = mysql_fetch_assoc($queryResult)) 
        {
            $return[$run] = $queryResultFetch;
            $run++;
         }
        
        return $return;
    }
    
    //Adds a new dataset
    function insert($into, $columns, $values)
    {
        $queryString = "INSERT INTO " . VIA_DB_PREFIX . $into . " (" . $columns . ") VALUES (" . $values . ")";
        
        $queryResult = mysql_query($queryString) or die(mysql_error());
        
        return $queryResult;
    }
} 

Aufgerufen wird sie dann z.B. so:

$db = new DBQuery();
$select = $db->select("*", "testTable",null,10);
$db->close(); 

Was haltet ihr davon?

Danke und Gruss

[asp2php]

1. deine Klasse ist à la PHP4 => nicht ratsam
2. sie ist von 'ner Menge externe Konstante abhängig => nicht gut
3. ORDER BY muss vor LIMIT
4. Keine eigene Fehlerbehandlung, or die() ist nicht professionell
5. $this->dbConnection wurde nicht angerührt, du gehst einfach davon aus, dass die Connection existiert
6. keine Überprüfung, ob alles OK ist bevor du Abfragen an die DB absetzst
7. ...
8. ...
9. ...
...

Fazit: die Klasse braucht niemand, ist gut für die Mülltone

[PHPbeginner]

Original geschrieben von asp2php
1. deine Klasse ist à la PHP4 => nicht ratsam
2. sie ist von 'ner Menge externe Konstante abhängig => nicht gut
3. ORDER BY muss vor LIMIT
4. Keine eigene Fehlerbehandlung, or die() ist nicht professionell
5. $this->dbConnection wurde nicht angerührt, du gehst einfach davon aus, dass die Connection existiert
6. keine Überprüfung, ob alles OK ist bevor du Abfragen an die DB absetzst
[...]

Hallo asp2php.

1. Wie meinst du das?
2. Es sind 5 Konstanten die in einem Config-File definiert werden. Was daran nicht gut sein soll versteh ich nun wirklich nicht...
3. Ja stimmt. Werd ich korrigieren.
4. Ja, eine eigene Fehlerbehandlung werde ich auch noch machen.
5. Wird doch über den Konstruktor gemacht...
6. Wozu? Der Programmierer der sie verwendet wird ja wohl wissen wie die Abfragen aussehen müssen??

[asp2php]

1. http://de3.php.net/manual/en/language.oop5.php lesen dann wirst du den Unterschied sehen
2. eine Klasse muss unabhängig von externen Dingen funktionieren, deine aber nicht => Hint: wenn du dabei bleibst, dann die Werte als Parameter des Konstruktors deklarieren
5. Klar, der SQL-Server läuft und läuft und läuft, oder was
6. SQL-Injection und Co, so wie Connection-Überprüfung muss der Progammierer alles selbst machen, oder was? Wozu braucht er noch eine Klasse?

[PHPbeginner]

1. Super, danke. Werde mir das noch mal genauer ansehen, erinnert mich schon sehr stark an C#.
2. Diese Klasse werd ich bestimmt sehr oft brauchen, daher hab ich keine Lust, jedes Mal die ganzen Konstanten zu übergeben. Oder hab ich das falsch verstanden?

5. Ja, genau so ist es. Ich werde trotzdem noch eine Exception einbauen.
6. Na gut, schau ich mir nochmals an.

[PHPbeginner]

So ich habe die ganze Klasse jetzt noch ein mal überarbeitet. Was jetzt noch fehlt ist die Überprüfung der Eingaben, aber das werde ich Morgen machen. Was haltet ihr (oder du asp2php ) davon?

class DBQuery
{
    //Variables
    private $__databasePrefix = VIA_DB_PREFIX;
    private $__connectionServer = VIA_DB_SERVER;
    private $__connectionUser = VIA_DB_USER;
    private $__connectionPassword = VIA_DB_PASSWORD;
    private $__connectionDatabase = VIA_DB_DATABASE;
    
    //Creates a new MySQL connection
    public function __construct()
    {
        $this->connectDB($this->__connectionServer, $this->__connectionUser, $this->__connectionPassword, $this->__connectionDatabase);
    }
    
    private function connectDB ($connServer, $connUser, $connPassword, $connDatabase)
    {
        try
        {
            $this->dbConnection = @mysql_connect($connServer, $connUser, $connPassword);
            
            if (!$this->dbConnection)
                throw new Exception;
        }
        catch (Exception $e)
        {
            //TODO: Handle an exception
        }
        
        if ($this->dbConnection)
        {
            try
            {
                $this->selectDB = @mysql_select_db($connDatabase);
                if (!$this->selectDB)
                    throw new Exception;
            }
            catch (Exception $e)
            {
                //TODO: Handle an exception
            }
        }
    }

    //Closes the open MySQL connection
    public function close()
    {
        try
        {
            $this->dbConnection = @mysql_close();
        }
        catch (Eception $e)
        {
            //TODO: Handle an exception
        }
    }
    
    //Returns a select
    public function select($what = '*', $from, $where = null, $limit = null, $orderby = null)
    {
        $return = null;
        
        if($this->dbConnection && $this->selectDB)
        {
            $queryString = "SELECT " . $what . " FROM " . $this->__databasePrefix . $from;
            
            if ($where != null)
                $queryString .= " WHERE " . $where;
            
            if ($orderby != null)
                $queryString .= " ORDER BY " . $orderby;
            
            if ($limit != null)
                $queryString .= " LIMIT " . $limit;
            
            try
            {
                $queryResult = @mysql_query($queryString);
                if(!$queryResult)
                    throw new Exception;
            }
            catch (Exception $e)
            {
                //TODO: Handle an exception
            }
            
            $run = 0;
            while ($queryResultFetch = @mysql_fetch_assoc($queryResult)) 
            {
                $return[$run] = $queryResultFetch;
                $run++;
             }

        }
        return $return;
    }
    
    //Adds a new dataset
    public function insert($into, $columns, $values)
    {
        $return = null;
        
        if($this->dbConnection && $this->selectDB)
        {
            $queryString = "INSERT INTO " . $this->__databasePrefix . $into . " (" . $columns . ") VALUES (" . $values . ")";
            
            try
            {
                $queryResult = @mysql_query($queryString);
                if(!$queryResult)
                    throw new Exception;
                else
                    $return = $queryResult;
            }
            catch (Exception $e)
            {
                //TODO: Handle an exception
            }
            
        }
            return $return;
    }
} 

[onemorenerd]

Bitte brich deinen Code um.

Die Klasse hängt noch immer von den externen Konstanten ab. So wäre es besser:

function __construct($server = null, $user = null, ...) {
    if (!is_null($server)) {
        $this->server = $server;
    } elseif (is_defined('DB_SERVER')) {
        $this->server = DB_SERVER;
    }
    // analog für die anderen Parameter
} 

[closure]

Hi,

wie formuliere ich damit joins oder wie kann ich aggregatfunktionen
benutzen?

Dein select ist nicht so schön weil es einfach zu sehr einschränkt.

Besser wäre es wenn deine klasse nur sicher die verbindung zur
datenbank herstellt und auch wieder schliessen kann.
Das sollte btw. auch automatisch geschehen wenn die klasse gc'ed
wird. Eine methode um rohe sql-queries abzusetzen.
Für queries würde ich weitere klassen verwenden und die hauptklasse
als delegator oder proxy für diese spezialisierten klassen einsetzen.

Die können sich dann um verschiedenen formen von queries kümmern
sowie um das richtige maskieren der eingabe.

Für die rückgaben würde ich mir auch etwas mehr oo-annehmlichkeiten
wünschen.

greets