[PHP] Nieuwe class initialiseren binnen andere class

[Vinze]

Ik loop nu al een tijdje tegen een probleem aan waar ik geen oplossing voor kan vinden.
Het probleem is als volgt:

Ik heb een class die heet Database hierin staan een aantal methods om gegevens uit de database te halen, deze methods worden door andere classes gebruikt als ze gegevens uit de database moeten hebben.

Een stukje van mijn database class:

PHP:

class Database {
    protected $db_link;
    public function __construct($db_link) {
        $this->db_link = $db_link;
    }
    public function Select($sql) {
        $query = $this->db_link->query($sql);
        if($query->num_rows) {
            while($row=$query->fetch_assoc()) {
                $data[]=$row;
            }
            return $data;
        }
        else {
            return false;
        }
    }
}

Wat ik voorheen deed was het volgende:

PHP:

class Music extends Database {
    public function Overview(){
        $sql="SELECT * FROM `tabel` LIMIT 1000";
        return $this->Select($sql);
    }
}

Oftewel, de class Music overerft Database om gebruik te maken van die Methods.
Nu ben ik er achter gekomen dat dit niet een nette manier van programmeren is, de Database class heeft namelijk niks de maken met de Music class.

Nu ben ik aan het zoeken wat wel netjes is, en heb ik het volgende gedaan:

PHP:

class Music {
    public function __construct($db_link) {
        $database = new Database($db_link);
    }
    public function Overview(){
        $sql="SELECT * FROM `tabel` LIMIT 1000";
        return $database->Select($sql);
    }
}

Ik dacht dat dit wel netjes was en zou werken, helaas krijg ik nu de foutmelding:
Fatal error: Call to a member function Select() on a non-object in pad../classes/Music.php
De classes worden geladen met me functie __autoload bovenaan de index.php.

Iemand enig idee waarom dit niet goed gaat, en wat de beste oplossing is voor mijn problemen?

[iBasch]

Kijk eens naar het Singleton-pattern:
http://php.net/manual/en/language.oop5.patterns.php

Samenvatting:
- Constructor is private
- Object ophalen met static methode
- Er kan maar één object, dus maar één gelijktijdige database-verbinding bestaan

[Down]

De foutmelding spreekt boekdelen. Hint: de variabele $database is niet beschikbaar in de functie Overview, omdat hij in de scope van je constructor staat.

[NMe]

...wat grappig is, aangezien je $this wel goed weet te gebruiken in je Music-class.

[Vinze]

Oh ja wat stom! Vandaag maar even genoeg geprogrammeerd, morgen met een fris hoofd weer verder

Oplossing voor als iemand dit topic ooit vind:

PHP:

class Music {
    protected $database;
    public function __construct($db_link) {
        $this->database = new Database($db_link);
    }
    public function Overview(){
        $sql="SELECT * FROM `tabel` LIMIT 1000";
        return $this->database->Select($sql);
    } 
}

@iBasch
Bedankt, dat ga ik morgen even doorlezen!

[mithras]

iBasch schreef op vrijdag 08 juli 2011 @ 22:14:
Kijk eens naar het Singleton-pattern:
http://php.net/manual/en/language.oop5.patterns.php

Samenvatting:
- Constructor is private
- Object ophalen met static methode
- Er kan maar één object, dus maar één gelijktijdige database-verbinding bestaan

Laten we hier geen singleton op toepassen. Het is niet waar bij een database (meerdere connecties zijn prima mogelijk) en niet handig (bijvoorbeeld bij unit tests).

Ik zou ook eerder dependency injection toepassen (wat hier in weze al gebeurt met de $db_link) dan het omschrijven naar een singleton

[iBasch]

mithras schreef op vrijdag 08 juli 2011 @ 22:58:
[...]
Laten we hier geen singleton op toepassen. Het is niet waar bij een database (meerdere connecties zijn prima mogelijk) en niet handig (bijvoorbeeld bij unit tests).

Ik bedoelde: Door het singleton-pattern te gebruiken limiteer je het aantal gelijktijdige connecties tot 1. In de meeste gevallen zijn er namelijk niet meer dan één nodig. In specifieke gevallen waar je meerdere connecties gebruikt zou ik inderdaad niet voor een singleton kiezen.

[Styxxy]

iBasch schreef op vrijdag 08 juli 2011 @ 23:08:
[...]
Ik bedoelde: Door het singleton-pattern te gebruiken limiteer je het aantal gelijktijdige connecties tot 1. In de meeste gevallen zijn er namelijk niet meer dan één nodig. In specifieke gevallen waar je meerdere connecties gebruikt zou ik inderdaad niet voor een singleton kiezen.

Dan zorg je er gewoon voor dat er slechts 1 instantie gemaakt wordt en dat die doorgegeven wordt. Totaal geen singleton voor nodig hoor...

[NMe]

mithras schreef op vrijdag 08 juli 2011 @ 22:58:
[...]
Laten we hier geen singleton op toepassen. Het is niet waar bij een database (meerdere connecties zijn prima mogelijk) en niet handig (bijvoorbeeld bij unit tests).

Bij een database denk ik inderdaad eerder aan een factory die een databaseclass van het juiste type aanmaakt.

[YopY]

Vinze schreef op vrijdag 08 juli 2011 @ 22:38:
Oh ja wat stom! Vandaag maar even genoeg geprogrammeerd, morgen met een fris hoofd weer verder

Oplossing voor als iemand dit topic ooit vind:

PHP:

class Music {
    protected $database;
    public function __construct($db_link) {
        $this->database = new Database($db_link);
    }
    public function Overview(){
        $sql="SELECT * FROM `tabel` LIMIT 1000";
        return $this->database->Select($sql);
    } 
}

@iBasch
Bedankt, dat ga ik morgen even doorlezen!

Random vraagje, waarom geef je de $db_link door ipv het Database object zelf? Moet de Music class een eigen Database instantie hebben? Welke velden zijn er anders in het Database object voor Music dan voor een ander model-object?

Dependency injection zoals dat is overigens wel baas .

[Vinze]

Ik maak in m'n config bovenaan de pagina verbinding met de database:

PHP:

$db_link = new mysqli("server","user","pass","database");
$db_link->select_db("database");

En deze gebruik ik dus ook in m'n classes om queries uit te voeren.
Dit zou ik wellicht ook direct in m'n Database class kunnen doen..

[NMe]

Wellicht? Niet lullig bedoeld, maar ik begrijp niet zo waar je DB-class voor dient als een DB-object niet zijn eigen connectie naar de database heeft.

[Anoniem: 26306]

iBasch schreef op vrijdag 08 juli 2011 @ 22:14:
Kijk eens naar het Singleton-pattern:
http://php.net/manual/en/language.oop5.patterns.php

Samenvatting:
- Constructor is private
- Object ophalen met static methode
- Er kan maar één object, dus maar één gelijktijdige database-verbinding bestaan

Nee!
Een singleton kan heel leuk zijn, maar moet enkel worden gebruikt voor iets waarvan er écht maar één van mag bestaan. Er is toch geen technisch bezwaar om meerdere databases open te hebben?
Hier een singleton aanraden is eigenlijk iemand op het verkeerde spoor zetten en dat moet je dus niet doen.

[kwaakvaak_v2]

Zolang je er maar voor zorgt dat elke connectie gesloten wordt, al dan niet automagisch. Maar als je beetje drukke site hebt, is het soms wel handig om per request maar een connectie open te hebben ipv voor elke query een nieuwe connectie te openen. En daar kan een singleton je wel degelijk bij helpen. Dus om nu te stellen dat een singleton het nieuwe kwaad is... naah

[mbarie]

Ik ben het met Cheatah eens.

Je kan per definitie meerdere databases hebben binnen 1 applicatie. Een singleton maken van je database classe is vragen om problemen als je een schaalbare oplossing zoekt.

Per pagerequest open je normaalgesproken een connectie met database X slechts 1 maal. Voor elke query een nieuwe connectie openen is vrij onlogisch. En dit probleem oplossen via een singleton klinkt meer als het stapelen van bad practices.

Wat ik me wel kan voorstellen is dat je een singleton hebt waarin de globale data van je applicatie bijgehouden wordt, met daarin je database instantie(s).

[kwaakvaak_v2]

mbarie schreef op zaterdag 09 juli 2011 @ 14:00:


Wat ik me wel kan voorstellen is dat je een singleton hebt waarin de globale data van je applicatie bijgehouden wordt, met daarin je database instantie(s).

hrmm als je in een singleton registry/global stack je database instantie initializeert is het nog steeds een singleton DB instance hoor, immers je hebt een referentie naar het object in je singleton en er mag per definitie van een singleton maar één instance van dat object leven.

Maar er is wel een verschil tussen een multiDB applicatie, en een applicatie met multiple connecties. Je kunt als je dat zou willen de singleton van je verschillende DB's onder verschillende namen registeren zodat je in je code ook leesbaar hebt waar wat vandaan komt.

[code]
$projecten= new db(connectionparams);
$gebruikers = new db(andereconnectieparams);

$projecten->select....
$gebruikers->update..

etc. etc. Dat iets een singleton is, betekend niet automagisch dat het dus ook statisch is..


(niet dat ik iemand het singleton patroon wil aanpraten, maar het is niet per saldo een slecht patroon, er bestaan hooguit slechte implementaties van het patroon )

[Anoniem: 26306]

kwaakvaak_v2 schreef op zaterdag 09 juli 2011 @ 14:12:
(niet dat ik iemand het singleton patroon wil aanpraten, maar het is niet per saldo een slecht patroon, er bestaan hooguit slechte implementaties van het patroon )

En databaseverbindingen zijn daarvan het beste voorbeeld.

code:

$projecten= new db(connectionparams);
$gebruikers = new db(andereconnectieparams);

$projecten->select....
$gebruikers->update..

Dit is dus duidelijk géén singleton.

[Gomez12]

kwaakvaak_v2 schreef op zaterdag 09 juli 2011 @ 13:13:
Dus om nu te stellen dat een singleton het nieuwe kwaad is... naah

Singleton is niet het nieuwe kwaad.
Singleton gebruiken voor iets wat niet altijd een singleton zal zijn, dat is gewoon een aloud kwaad.

[Voutloos]

kwaakvaak_v2 schreef op zaterdag 09 juli 2011 @ 13:13:
Zolang je er maar voor zorgt dat elke connectie gesloten wordt, al dan niet automagisch.

Gaat automagisch al goed genoeg voor iedereen. Op het moment dat het een probleem wordt heb je al fulltime seniors met genoeg verstand van zaken voor trucjes.

Maar als je beetje drukke site hebt, is het soms wel handig om per request maar een connectie open te hebben ipv voor elke query een nieuwe connectie te openen.

Connecties hergebruiken waar mogelijk is logisch, en standaard, maar heeft echt niet direct met een pattern als singleton te maken.

En daar kan een singleton je wel degelijk bij helpen.

Nope singleton dwingt je tot 1 connectie met maar 1 database in totaal. Dwingen/beperken != helpen.

Dependency injection (en optioneel gerelateerde begrippen als service locator/registry) is hier het keyword. TS was al op wezig met het doorgeven van $db_link, zodat niet iedere class zelf hoeft te weten hoe ze een connectie moeten opzetten.
De verbetering is eenvoudig: Laat je database class de connectie opzetten en geef dat ding door (of haal uit Registry of whatever), en al je classes hoeven direct niet meer te weten hoe ze een Database moeten constructen. En dus wordt het ook mogelijk om je Database te mocken of te subclassen zonder dat je al je code hoeft aan te passen.

[mithras]

Haha, leuk die discussie over singletons en DI. Volgens mij was de TS al geholpen met de eerste paar posts. Sowieso pastte TS hier al min of meer DI toe, alleen wist hij het (waarschijnlijk) niet. En op een nét wat onhandige manier zodat je niet kan type hinten naar Database, maar dat komt wel goed denk ik

Zullen we nu gaan aanbevelen om liever een entitymanager te pakken dan een sort-of active record toe te passen?

[kwaakvaak_v2]

Anoniem: 26306 schreef op zaterdag 09 juli 2011 @ 14:28:
[...]

En databaseverbindingen zijn daarvan het beste voorbeeld.

[...]

Dit is dus duidelijk géén singleton.

Hoezo niet? De database class + connectie bestaat maar één keer in deze vorm, en dus een singleton. Dat er meerdere instanties van een singleton aktief zijn, wil niet zeggen dat het geen singleton volgens het patroon is.

Tuurlijk kun je stellen dat het ook een factory patroon is, maar het resultaat van een factory kan best een singleton zijn. Niet dat het een goed idee, maar ik begrijp de hele hetze tegen het singlepatron. Maar het is blijkbaar hip om niet meer te gebruiken op dit moment in PHP land.

[Voor 24% gewijzigd door kwaakvaak_v2 op 09-07-2011 19:27]

[Anoniem: 26306]

kwaakvaak_v2 schreef op zaterdag 09 juli 2011 @ 19:23:

Hoezo niet? De database class + connectie bestaat maar één keer in deze vorm, en dus een singleton. Dat er meerdere instanties van een singleton aktief zijn, wil niet zeggen dat het geen singleton volgens het patroon is.

Als er meerdere instanties mogelijk zijn is het geen singleton. Moet ik het echt gaan uitleggen of houd je op met voor lul staan en ga je er zelf eens wat over lezen?

Tuurlijk kun je stellen dat het ook een factory patroon is, maar het resultaat van een factory kan best een singleton zijn. Niet dat het een goed idee, maar ik begrijp de hele hetze tegen het singlepatron.

Maar het is blijkbaar hip om niet meer te gebruiken op dit moment in PHP land.

Goed dat je dat zegt, want ik heb een vaag vermoeden dat je eigenlijk alleen maar PHP kent.

Tip. Ga eens bijvoorbeeld eens een Java servlet maken met een database connection pool. Ga dan nog eens goed nadenken over je singleton* en waarom dat dan toch niet zo'n goed idee was.

* voorwaarde is natuurlijk dat je dan wel begrijpt wat een singleton is

[NMe]

Anoniem: 26306 schreef op zaterdag 09 juli 2011 @ 19:38:
[...]

Als er meerdere instanties mogelijk zijn is het geen singleton.

offtopic:
Dan maak je toch gewoon een doubleton?

Laten we trouwens iets dimmen wat betreft commentaar op anderen. Ik ben de laatste die wat tegen een stevige discussie heeft, maar laten we het wel constructief houden.

[krvabo]

Anoniem: 26306 schreef op zaterdag 09 juli 2011 @ 14:28:
[...]

En databaseverbindingen zijn daarvan het beste voorbeeld.

Volgens mij is je architectuur nog altijd een verlengde van je (functionele) eisen.
Stel dat ik een programma wil maken waarbij altijd maar _één_ databaseconnectie gemaakt hoeft worden gemaakt. Altijd. Geen enkel vooruitzicht op ooit een tweede database.

Op dat moment is een singleton in je databaseklasse een valide keuze.

Ik zeg dus niet dat het dan een verplichting is, maar het afdoen als kutcode is ook zowat.

[Voor 4% gewijzigd door krvabo op 10-07-2011 03:07]

[NMe]

Functionele eisen stel je op dit niveau op basis van dingen die moeten, niet dingen die hoeven. Als het geen kwaad kan om een tweede connectie te maken maar ik er gewoon nooit meer dan één nodig heb, dan kies ik nog niet voor een singleton. Als het onmogelijk moet zijn omdat er bijvoorbeeld anders mogelijk concurrencyproblemen kunnen optreden of omdat het openen van meerdere simultane connecties performanceproblemen heeft, dan kies ik daar wel voor. Subtiel verschil, maar wel belangrijk.

[krvabo]

NMe schreef op zondag 10 juli 2011 @ 03:11:
Functionele eisen stel je op dit niveau op basis van dingen die moeten, niet dingen die hoeven.

MoSCoW.
Verder stond 'Functionele' tussen haakjes.

Als het geen kwaad kan om een tweede connectie te maken maar ik er gewoon nooit meer dan één nodig heb, dan kies ik nog niet voor een singleton.

Sja, ik wel, maar het ligt er ook aan voor wat. Bij webapplicaties kies ik vaker voor een singleton DB-klasse dan in een desktop app. Voor de webapplicaties die ik maak weet ik dat voor de voorzienbare toekomst (de levensspan van de versie van de site) nooit een tweede databaseverbinding hoef te hebben en daardoor de mogelijkheid ook niet wil hebben, zodat het "makkelijker" te programmeren is.

[kwaakvaak_v2]

Of ik meer dan PHP kan, staat los van deze discussie imho, het gaat immers in deze discussie over het gebruik van het singleton patroon in PHP. Er is een subtiel maar niet onbelangrijk verschil tussen theoretische toepassing van een patroon en een praktische implementatie binnen een bepaald domein en taal. En voor de goede orde, ik beweer ook nergens dat het een goed principe is, om binnen een applicatie meerdere instanties van een DB laag te gebruiken (zie verder naar beneden).

Maar het is iets wat met query class binnen PHP wel kan. Puur omdat een DB laag in PHP eigenlijk meer een class is om op één centrale plek je SQL naar data hydratatie af te handelen en vaak als wrapper class om pdo/mysql gebruikt wordt om ofwel een tekortkoming in in eenduidige syntax/connectie strings (pdo) of het eindeloos voorkomen van while ($row = mysql_get_row..) loops in je code (mysql) af te handelen. En dus in combinatie met je connectie string dermate uniek is dat het een singleton zou kunnen zijn.

Dat het slimmer is om met domain driven data te werken is een ander verhaal, maar dat is denk ik voor de TS op dit moment een aantal bruggen te ver

Wat krvabo hier boven ook al vertelde, als je van te voren weet dat er in de levensduur van je web applicatie er nooit meer dan 1 DB connectie gebruikt gaat worden waarom dan heel erg moeilijk gaan doen om een bepaald patroon te moeten vermeiden puur uit principe omdat het misschien wel eens zou kunnen gebeuren dat er misschien dit of dat nodig gaat zijn, met als gevolg dat je overbodige code gaat schrijven, of je applicatie dermate aan het over-enginering bent dat het volledige doel zoek is. Terwijl je code er een stuk leesbaarder op wordt als je gewoon binnen een functie $data = getDB()->getrows('SELECT FROM...) kunt gebruiken en klaar.

De ervaring leert immers, dat op het moment dat wanneer je een tweede DB connectie nodig hebt in je applicatie het 10 van de 10 gaat om externe applicatie vreemde data gaat. En dan is het veel slimmer om die data bron als volledig nieuwe service laag te introduceren in een eigen object ipv het er met een dubbele connectie in te knoeien. Abstractie ipv spaghetti knoeien en dat principe is imho volledig taal onafhankelijk.

Dus in dit daglicht zijn DP's zijn meer een leidraad dan een heilig boek wat kost dat kost gevolgd moet worden onafhankelijk of het nuttig is of niet.

[Voutloos]

Lang verhaal maar het gaat jou blijkbaar toch maar om 1 ding: Dat je de/een Database instance er makkelijk bij kan pakken. En daar heb je niet per se singleton voor nodig.

Ik noemde al een aantal termen, en aangezien TS al enigszins dependency injection deed is het onzinnig om met singleton aan te komen zetten.

[Voor 29% gewijzigd door Voutloos op 10-07-2011 12:09]

[kwaakvaak_v2]

Dat klopt je hebt de singleton idd niet nodig, sterker nog ik kan mijzelf niet eens herinneren wanneer ik voor het laatst een singleton iets gemaakt heb. Maar het moet zeker een jaar of twee geleden zijn.

Het ging mij meer om het principe dat beweert werd dat singleton === evil

[NMe]

kwaakvaak_v2 schreef op zondag 10 juli 2011 @ 13:46:
Dat klopt je hebt de singleton idd niet nodig, sterker nog ik kan mijzelf niet eens herinneren wanneer ik voor het laatst een singleton iets gemaakt heb. Maar het moet zeker een jaar of twee geleden zijn.

Het ging mij meer om het principe dat beweert werd dat singleton === evil

Niemand heeft beweerd dat een singleton evil is, er is hierboven alleen gezegd dat het stom is om hem toe te passen waar hij niet strikt noodzakelijk is. Iets met the right tool for the right job.