[PHP] Alternatief voor Singleton - meerdere databases

[TheJVH]

Op dit moment ben ik bezig met een webapplicatie die gebruik maakt van 2 databases. In mijn eerdere applicaties heb ik altijd gebruik gemaakt van een Singleton pattern die voor alle database interactie zorgt.

Het grote nadeel hiervan is dat een Singleton pattern geen oplossing bied voor het werken met meerdere databases.

Mijn database class ziet er op dit moment zo uit, het werkt voor mij erg prettig, op deze manier kan ik bijvoorbeeld snel wisselen tussen het updaten en het inserten van data, zonder de data zelf aan te passen, daar zorgt deze class namelijk voor.

PHP:

<?php
/*
 * database class for accessing the database. Uses MySQLi for the connection, can be accessed by a Singleton pattern.
 * The connection information is also stored here.
 */

class database {
    
    private static $instance;
    
    private $db_host = 'localhost';
    private $db_name = 'intranet';
    private $db_user = 'root';
    private $db_pass = '';
    private $hash = 'blaat';
    
    public $mysqli;
    
    private function __construct(){
        $this->mysqli = new mysqli($this->db_host, $this->db_user, $this->db_pass, $this->db_name);
        if ($this->mysqli->connect_errno) {
            echo "Failed to connect to MySQL: " . $this->mysqli->connect_error;
            exit();
        }
        $this->mysqli->autocommit(TRUE);
        $this->mysqli->query("SET NAMES 'utf8';");
    }
    
    public static function getInstance(){
        if (!isset(self::$instance))
        {
            self::$instance = new database();
        }
        
        return self::$instance;
    }
    
    /*
     * Custom functions for easier development
     */
    
    // Fetch a single row
    public function fetchSingle($where, $values, $table){
        if($result = $this->select($where.' LIMIT 1', $values, $table)){
            return $result;
        } else {
            return false;
        }
    }
    
    // Returns the number of affected rows
    public function numRows($where, $table){
        if($this->mysqli->query('SELECT id FROM '.$table.' WHERE '.$where)){
            return $this->mysqli->affected_rows;
        } else {
            echo $this->mysqli->error;
            return false;
        }
    }
    
    // Returns true if there is only 1 row affected, else false
    public function unique($where, $table){
        if($this->numRows($where, $table) == 1){
            return true;
        } else {
            return false;
        }
    }
    
    // This function outputs a hashed string used for passwords.
    public function password($password){
        return sha1(sha1($password).$this->hash);
    }
    
    /*
     * Default SQL functions, they use the same input and output so it's easy to switch between them (inserting / updating).
     * Remember to use a $db->mysqli->real_escape_string() before using these functions to prevent database insertion.
     */
     
    public function select($where, $table, $select = array('*'), $orderby = 'id', $limit = false){
        // Create a string to be used in the query with all the data
        $select_string = implode(', ', $select);
        
        if(!$limit){
            $limitstr = '';
        } else {
            $limitstr =  ' LIMIT '.$limit;
        }
        //echo 'SELECT '.$select_string.' FROM '.$table.' WHERE '.$where.' ORDER BY '.$orderby.$limitstr.'<br/>';
        if($result = $this->mysqli->query('SELECT '.$select_string.' FROM '.$table.' WHERE '.$where.' ORDER BY '.$orderby.$limitstr)){
            if($result->num_rows){
                while ($row = $result->fetch_assoc()) {
                    $return[] = $row;
                }
                return $return;
            } else {
                return false;
            }
        } else {
            echo $this->mysqli->error;
            return false;
        }
    }
    
    // Returns true on succes, or false on failure.
    public function update($where, $values, $table){
        // Create a string to be used in the query with all the data
        foreach($values as $key => $value){
            $value_array[] = $key.' = "'.$value.'"';
        }
        $value_string = implode(', ', $value_array);
        if($this->mysqli->query('UPDATE '.$table.' SET '.$value_string.' WHERE '.$where)){
            return true;
        } else {
            echo $this->mysqli->error;
            return false;
        }
    }
    
    public function delete($where, $table){
        if($this->mysqli->query('DELETE FROM '.$table.' WHERE '.$where)){
            return true;
        } else {
            echo $this->mysqli->error;
            return false;
        }
    }
    
    public function insert($values = array(), $table){
        // Create a value and key string, used in the query
        foreach($values as $key => $value){
            $value_array[] = $value;
            $key_array[] = $key;
        }
        $value_string = implode('", "', $value_array);
        $key_string = implode("`, `", $key_array);
        
        if($this->mysqli->query('INSERT INTO '.$table.' (`'.$key_string.'`, date_created, date_modified) VALUES ("'.$value_string.'", NOW(), NOW())')){
            return $this->mysqli->insert_id;
        } else {
            echo $this->mysqli->error;
            return false;
        }
    }

}

Het werkt voor mij erg prettig, ik kan het bijvoorbeeld in classes gebruiken waarmee ik formulieren verwerk (weergeven op het scherm, valideren en het processen naar de database).

Helaas heb ik zelf nooit echt les gehad in OOP en ben ik dus zelf bezig met het leren ervan, vooral door het veel te doen. Ik weet niet of deze methode uberhaupt goed is, of dat er misschien een hele andere manier (of denkwijze) is om dit aan te pakken (zoals een functie in een class die een SQL query teruggeeft).

Zijn er ergens goede naslagwerken die de architectuur van OOP in PHP uitleggen? Dus niet het opbouwen van classes en dergelijke, maar meer de gedachte erachter. Wanneer je bijvoorbeeld een class aanmaakt, wat deze doet, hoe je deze aan elkaar koppelt, etc.

EDIT: Het belangrijkste is dus dat ik bij m'n database class kan in een andere class. Singletons voldoen hieraan, maar dat werkt nu dus niet omdat ik m'n database class meerdere keren wil instantieren .

[sky-]

Je zou eens kunnen kijken naar een Factory pattern (http://www.oodesign.com/factory-pattern.html).

Er zijn (genoeg) boeken over OO. Specifiek php weet ik zo niet.

Hier worden wel wat OO dingen behandeld: http://www.codeproject.co...iented-Programming-Concep

Wat betreft patterns, je zou wel eens hier kunnen kijken ter inspiratie: http://css.dzone.com/books/practical-php-patterns/. Deze patterns staan ook in het boek 'Patterns of Enterprise Application Architecture' van Martin Fowler. Prima boek.

[ Voor 17% gewijzigd door sky- op 23-10-2012 15:19 ]

[Zoijar]

Het simpelste om het uit te breiden is denk ik om ipv een enkele $instance var daar een array van te maken, en in de getInstance() een integer parameter toevoegen welke instance je wilt, eventueel met default 0. Dan krijg je dus iets als database::getInstance(1).select() en if (isset(self::instance[1]) etc.

[Hydra]

Ik zie sowieso het nut niet zo van een Singleton als je met request-scope PHP werkt. Daarnaast zijn er nog een paar issues: ten eerste staat de configuratie in de code, ten tweede ziet de code er niet erg SQL-injection proof uit.

Maak gewoon X instances van je class aan, eentje voor elke database instance die je wil gebruiken en geef gewoon de connectstring mee. Die haal je dan 'ergens' uit een config file.

Zoijar schreef op dinsdag 23 oktober 2012 @ 16:05:
Het simpelste om het uit te breiden is denk ik om ipv een enkele $instance var daar een array van te maken, en in de getInstance() een integer parameter toevoegen welke instance je wilt, eventueel met default 0. Dan krijg je dus iets als database::getInstance(1).select() en if (isset(self::instance\[1]) etc.

Een database class die meerdere databases bedient is natuurlijk uit OO oogpunt hardstikke ranzig.

[ Voor 38% gewijzigd door Hydra op 23-10-2012 16:13 ]

[Grijze Vos]

Zoijar schreef op dinsdag 23 oktober 2012 @ 16:05:
Het simpelste om het uit te breiden is denk ik om ipv een enkele $instance var daar een array van te maken, en in de getInstance() een integer parameter toevoegen welke instance je wilt, eventueel met default 0. Dan krijg je dus iets als database::getInstance(1).select() en if (isset(self::instance\[1]) etc.

Een multleton!

[Woy]

Ik snap sowieso niet waarom je hiervoor een Singleton zou willen gebruiken. Waarom niet gewoon een global var naar een instance van je Database class die je aan het begin van je script initialiseerd? Dan kun je voor een tweede database gewoon een tweede variabele aan.

[Morax]

Woy schreef op dinsdag 23 oktober 2012 @ 16:18:
Ik snap sowieso niet waarom je hiervoor een Singleton zou willen gebruiken. Waarom niet gewoon een global var naar een instance van je Database class die je aan het begin van je script initialiseerd? Dan kun je voor een tweede database gewoon een tweede variabele aan.

Ik gebruik persoonlijk het beste van twee werelden:
Mijn database class heeft een getInstance() functie die een instantie naar de default DB aanmaakt en teruggeeft (en cached voor de rest van de request).

Los daarvan heeft de class gewoon een public constructor zodat je zelf waar nodig extra verbindingen kan maken

[Hydra]

M.i. doe je het een of het ander. Wat jij doet is een halfbakken factory pattern.

[TheJVH]

Hydra schreef op dinsdag 23 oktober 2012 @ 16:12:ten tweede ziet de code er niet erg SQL-injection proof uit.

Klopt, en dat is hij bewust niet. Ik wil namelijk ook nog SQL functies als "NOW()" kunnen gebruiken in m'n code. Die sanitizing voer ik in m'n form classes uit .

Ik denk dat het beste idee nog wel de global variable is, dan blijft het relatief overzichtelijk .

Punt is dat ik nog nooit een andere taal heb geleerd dan PHP of JavaScript, dus ik ben niet goed thuis in OOP. Dat probeer ik te leren maar dat is best lastig als je dat nog nooit is uitgelegd (in termen van architectuur en dergelijke).

Thanks voor de reacties in ieder geval, zolang ik er van leer ervaar ik het allemaal positief .

[ Voor 6% gewijzigd door TheJVH op 23-10-2012 16:54 ]

[Kapitein Iglo]

Op het web wordt er vooral veel gesproken over dependency injection als singleton vervanger.
http://fabien.potencier.o...t-is-dependency-injection

[Grijze Vos]

Jij werkt toch niet aan het EPD of Digid he?

[Hydra]

TheJVH schreef op dinsdag 23 oktober 2012 @ 16:54:
Klopt, en dat is hij bewust niet. Ik wil namelijk ook nog SQL functies als "NOW()" kunnen gebruiken in m'n code. Die sanitizing voer ik in m'n form classes uit .

Er is geen enkele reden om geen PDO te gebruiken t.o.v. queries concatten, ook niet het gebruik van NOW(). Het hele idee is juist dat iedere dev fouten maakt (ook jij) en queries concatten gewoon inherent onveilig is. Komt bij dat string velden enorm lastig te controleren zijn op SQL injection.

[TheJVH]

Grijze Vos schreef op dinsdag 23 oktober 2012 @ 17:06:
Jij werkt toch niet aan het EPD of Digid he?

Gelukkig niet, maar waarom deze opmerking?

Hydra schreef op dinsdag 23 oktober 2012 @ 17:09:
[...]


Er is geen enkele reden om geen PDO te gebruiken t.o.v. queries concatten, ook niet het gebruik van NOW(). Het hele idee is juist dat iedere dev fouten maakt (ook jij) en queries concatten gewoon inherent onveilig is. Komt bij dat string velden enorm lastig te controleren zijn op SQL injection.

Maar als ik $db->mysqli->real_escape_string() gebruik, dan wordt mijn string toch op een goede manier opgemaakt voor de database?

EDIT: En ik snap dat ik beter prepared statements kan gebruiken, maar daar ben ik nog over aan het inlezen .

[ Voor 64% gewijzigd door TheJVH op 23-10-2012 17:22 ]

[Herko_ter_Horst]

TheJVH schreef op dinsdag 23 oktober 2012 @ 17:13:
[...]


Gelukkig niet, maar waarom deze opmerking?

Waarschijnlijk vanwege je opmerking over "sanitizen doe ik wel even in de form". Zo'n aanpak hoort niet thuis in productiecode (of ergens anders, maar voor je hobbyprojectje moet je zelf weten hoe groot je de puinhoop maakt ).

[...]


Maar als ik $db->mysqli->real_escape_string() gebruik, dan wordt mijn string toch op een goede manier opgemaakt voor de database?

Opgemaakt wel, veilig niet. Zie onder meer: http://ilia.ws/archives/1...-Prepared-Statements.html en http://johnroach.info/201...op-sql-injection-attacks/ (deze laatste ziet het probleem, maar gebruikt de "verkeerde" oplossing: van de programmeur verwachten dat hij nooit meer vergeet quotes te gebruiken is niet de oplossing; prepared Statements wel).

[ Voor 37% gewijzigd door Herko_ter_Horst op 23-10-2012 17:33 ]

[Morax]

Hydra schreef op dinsdag 23 oktober 2012 @ 16:41:
M.i. doe je het een of het ander. Wat jij doet is een halfbakken factory pattern.

Ik vind dit de ideale oplossing. Mede omdat je bij een global variabele in je Registry geen type hinting kan geven wat voor object erin zit. Nu kan ik netjes teruggeven dat de getInstance() method een instantie van het Database object teruggeeft, en kan mijn IDE netjes alle functies en dergelijke tonen

[Zoijar]

Grijze Vos schreef op dinsdag 23 oktober 2012 @ 16:16:
Een multleton!

Het klinkt misschien vreemd, maar dit is een van de use-cases die wordt aangehaald in het Design Patterns boek: het voordeel van een singleton boven een static is dat je later alsnog kan switchen naar meerdere instanties. "Consequences: 4: permits a variable number of instances (p128)" (even los van alle discussie of dit nou wel een goed idee is zo)

[ Voor 7% gewijzigd door Zoijar op 24-10-2012 11:07 ]

[Hydra]

Morax schreef op dinsdag 23 oktober 2012 @ 18:36:
Ik vind dit de ideale oplossing. Mede omdat je bij een global variabele in je Registry geen type hinting kan geven wat voor object erin zit. Nu kan ik netjes teruggeven dat de getInstance() method een instantie van het Database object teruggeeft, en kan mijn IDE netjes alle functies en dergelijke tonen

Als je 2 instanties hebt die eigenlijk volledig identiek zijn dan is het niet logisch dat een van beiden de 'default' is. Dat leidt eerder gewoon tot fouten. Je hebt 2 databases dus je hebt gewoon 2 object instanties, klaar.

Zoijar schreef op woensdag 24 oktober 2012 @ 11:01:
Het klinkt misschien vreemd, maar dit is een van de use-cases die wordt aangehaald in het Design Patterns boek: het voordeel van een singleton boven een static is dat je later alsnog kan switchen naar meerdere instanties. "Consequences: 4: permits a variable number of instances (p128)"

Alleen jammer dat jouw oplossing gewoon een verkeerde implementatie is van het idee dat je een singleton op kunt heffen. Jij bouwt IN een singleton meerdere objecten.

Daarnaast is dat het beter is dan alles static maken sowieso geen argument voor het gebruik van een singleton pattern voor iets waar het niet voor bedoeld is. Vrijwel alles is beter dan alles static maken.

[Grijze Vos]

Zoijar schreef op woensdag 24 oktober 2012 @ 11:01:
[...]

Het klinkt misschien vreemd, maar dit is een van de use-cases die wordt aangehaald in het Design Patterns boek: het voordeel van een singleton boven een static is dat je later alsnog kan switchen naar meerdere instanties. "Consequences: 4: permits a variable number of instances (p128)" (even los van alle discussie of dit nou wel een goed idee is zo)

Dat weet ik, dat boek ligt ook op mijn bureau. Ik vond het wel grappig dat je het aandroeg. Maar de "echte" oplossing hier is gewoon een factory natuurlijk, geen singleton, en geen static.