Rakennan omaan käyttööni pientä sisällönhallintajärjestelmää ja tavoitteena on samalla oppia hiukan syvemmin PHP:n olio-ohjelmointia. Katsoin hiukan Joomlasta mallia miten siellä päästään käsiksi niin suureen määrään luokkia eri paikoista koodia. Niinpä loin samanlaisen factory.php tiedoston, joka pelkistettynä näyttää tältä:
<?php
/**
* Factory abstract
*/
abstract class Factory {
/**
* Global user object
*
* @var User
*/
private static $user;
/**
* Global logs object
*
* @var Logs
*/
private static $logs;
/**
* Global component object
*
* @var Component
*/
private static $component;
/**
* Get a user object.
* Returns the global object, only creating it if it doesn't already exist.
*
* @return User object
*/
public static function getUser() {
if (!self::$user)
self::$user = self::createUser();
return self::$user;
}
/**
* Get a logs object.
* Returns the global object, only creating it if it doesn't already exist.
*
* @return Logs object
*/
public static function getLogs() {
if (!self::$logs)
self::$logs = self::createLogs();
return self::$logs;
}
/**
* Get a component object.
* Returns the global object, only creating it if it doesn't already exist.
*
* @return Component object
*/
public static function getComponent() {
if (!self::$component)
self::$component = self::createComponent();
return self::$component;
}
/**
* Create a user object
*
* @return User object
*/
protected static function createUser() {
return LUser::getInstance();
}
/**
* Create a logs object
*
* @return Logs object
*/
protected static function createLogs() {
return LLogs::getInstance();
}
/**
* Create a component object
*
* @return Component object
*/
protected static function createComponent() {
return LComponent::getInstance();
}
}Ongelma on, että jostain syystä luodessani Component olion ensimmäistä kertaa, se ei jää muistiin vaan seuraavalla getComponent-kutsulla olio luodaan uudestaan. Tämä tapahtuu sisäänkirjautuessa, kun yritän tallentaa kirjautumista lokiin. Kaikkialla muualla koodissa, kun kutsun getComponent metodia, metodi toimii ongelmitta eikä yritä luoda kyseistä oliota uudestaan, kun se on jo olemassa. Ainoastaan Logs olion sisällä oleva getComponent kutsu luo Component olion uudestaan.
Koodin toiminta yksinkertaistettuna on seuraava:
Minulla on login-komponentti, joka luo sivulle lomakkeen sisäänkirjautumista varten ja lähettää nämä lomakkeen tiedot User oliolle, joka käsittelee sisäänkirjautumisen. Kun sisäänkirjautuminen on onnistunut, User pyytää Logs oliota tallentamaan uuden loki-merkinnän. Tämä Logs olio sitten hakee tarvittavia tietoja uutta loki-merkintää varten. Yksi tieto on käytössä olevan komponentin id ja siksi se kutsuu tätä getComponent metodia saadakseen kyseisen tiedon. Jostain syystä kyseistä Component oliota ei kuitenkaan löydy muistista, jolloin se luodaan uudestaan ja koska se luodaan uudestaan, niin koodi menee loopiin. Tämä uusi Component olio luo uuden login-komponentin, joka jälleen käsittelee sisäänkirjautumislomakkeen ja lähettää tiedot uudestaan User oliolle, joka yrittää uudestaan luoda loki-merkintää jne. Sivu kiertää tätä loopia kuusi kertaa kunnes seitsemännellä kierroksella se tuntemattomasta syystä onnistuu tulemaan siitä ulos ja jatkaa koodin loppuun.
Olen koettanut nyt kaksi päivää debugata koodiani ja selvittää mistä tämä ongelma johtuu. Lopulta selvitin ongelman alkulähteen tähän Logs olioon, joka luo Component olion yhä uudestaan ja uudestaan, mutten ymmärrä mistä syystä näin käy.
Tässä Logs ja User oliot pelkistettynä:
<?php
/**
* User base object
*/
class LUser {
/**
* Instance container
*
* @var User
*/
private static $instance = null;
public function __construct() {
// Check if user is logged in, get his information and renew session and cookie if needed
$id = $this->loggedInWithSession() ?: $this->loggedInWithCookie();
// If user is not logged in, we set up default public account
$this->userInfo($id);
}
/**
* Returns the global User object, only creating it if it doesn't already exist.
*
* @return User The user object
*/
public static function getInstance() {
if (self::$instance === null) {
self::$instance = new LUser();
}
return self::$instance;
}
/**
* Handles login and sets session, and cookie if necessery.
*
* @return false if login failed, otherwise redirects to another page
*/
public function login() {
// Check that username exists
// Check that password was given
// Check if password was correct
// Add new session
// Update password hash
// Add new cookie if needed
// Set up user information
$this->userInfo(intval($user['id']));
// Add log about the login
Factory::getLogs()->add('user_login', 'loggedin');
// Redirect to home page
header('Location: ' . Factory::getUrl()->home());
die();
}
}<?php
/**
* Logs base object
*/
class LLogs {
/**
* Instance container
*
* @var Logs
*/
private static $instance = null;
public function __construct() {
}
/**
* Returns the global Logs object, only creating it if it doesn't already exist.
*
* @return Logs The logs object
*/
public static function getInstance() {
if (self::$instance === null) {
self::$instance = new LLogs();
}
return self::$instance;
}
/**
* Adds new log into database
*
* @param string $target What is the target of the log
* @param string $action What is the action that happened
* @param int $id User id (optional)
* @return bool True if log was added successfully, false on failure
*/
public function add($target, $action, $id = NULL) {
if (!is_string($action) || !is_string($target))
return false;
// Tämä getComponent kutsu luo uuden Component olion yhä uudestaan ja uudestaan
$component = Factory::getComponent()->get()->value('id');
$userid = $id === NULL ? Factory::getUser()->value('id') : $id;
try {
Factory::getDbo()->beginTransaction();
// Do some checks and add log to the database
$success = Factory::getDbo()->commit();
} catch (PDOException $e) {
Factory::getDbo()->rollBack();
Factory::getError()->set(LLang::_('FAILED_LOG: ', $e->getMessage()), 1);
$success = false;
}
return $success;
}
}Onko kenelläkään mitään ajatusta mistä suunnasta tätä vikaa voisi lähteä debuggaamaan? Minulta on keinot ja osaaminen loppunut tässä kohtaa.
Virhe ei varmasti ole se, että olio ”ei jää muistiin”, vaan luultavasti koodisi rakenne on sellainen, että oliota yritetään hakea uudestaan jo ennen sen ensimmäistä tallentamista. (Tai sitten olet sekoittanut jotain ja kyseessä onkin oikeasti useampi eri sivunlataus.)
Veikkaan, että jonkin __construct-funktion sisältö aiheuttaa syklisen riippuvuuden luokkien välille. Esimerkiksi tuossa LUser::__construct kutsuu jo useampaa eri funktiota. LUser-oliota ei ole silloin vielä tallennettu. Jos noista funktioista päädytään tilanteeseen, jossa yritetään hakea LUser (esimerkiksi lokimerkintää varten), sitä ei siis löydy ja se luodaan uudestaan. Toinen kysymys onkin, mihin tämä rekursiosilmukka sitten päättyy; luultavasti sattumalta johonkin tietokantavirheeseen tms.
Mitä ihmeen hyötyä tuosta Factory-viritelmästä mielestäsi on? Tulee vain ihan pöljää toistoa, kun yhden asian tekemiseen on tarvitaan Factory-luokassa getX ja createX ja sitten vielä X::getInstance. Tuollaisen koodin tarkoitus tuntuu olevan se, että globaali muuttuja maagisesti muuttuu jotenkin paremmaksi, kun se piilotetaan tarpeeksi monen funktiokutsun sisään.
Jos oliot ovat kuitenkin tuolla tavalla globaaleja (eli luokkaan on kovakoodattu se riippuvuus, olkoon globaali $x tai Factory::getX()), olisi selvempää, jos esimerkiksi SiteGlobals::getX sisältäisi tuon luomis- ja säilytyslogiikan ilman ylimääräisiä kerroksia createX ja getInstance. Lopputulos on aivan sama, eli luokkiin on edelleen kovakoodattu viittauksia näihin globaaleihin olioihin. Lisäksi nimi SiteGlobals kertoisi selvemmin, mistä on kyse.
Jos sitten oikeasti on tarkoitus tehdä systeemi, jossa esimerkiksi automaattisia testejä varten voidaan korvata jokin luokka palikkaversiolla, tähän löytyy varmasti selvempiäkin ratkaisuja.
En osannut ajatella, ettei oliota tosiaan ole vielä tallennettu siinä vaiheessa, kun kutsutaan __construct-funktiota. Sehän se olikin se ongelman ydin! Seurasin tuota Component oliota eri __construct funktioiden läpi ja totta tosiaan sitä kautta päädyttiin tuonne Logs luokan add funktioon, joka kutsui getComponent funktiota saadakseen Component olion, jota ei siis oltu vielä tallennettu. Ongelman ydin on nyt siis löytynyt. Enää pitää pohtia, että miten saan sen ratkaistua, jota en ainakaan näin nopeasti keksi. Minun pitänee katsoa tätä koodia isona kokonaisuutena ja tutkia miten lomakkeen tiedot käsittelisi järkevästi __construct:n ulkopuolella sen jälkeen kun kaikki tarvittavat oliot on ensin asennettu.
Mitä tulee tuohon "Factory-viritelmään", olen itsekin pohtinut samaa kysymystä. Mitä hyötyä tuosta on? Neljä vuotta sitten aloitellessani tätä projektia minulla ei ollut mitään käsitystä miten olisin lähtenyt liikkeelle kaikkien näiden luokkien kanssa ja saanut ne toimimaan järkevästi yhteen. Niinpä ilman parempaa tietoa tutkin miten Joomla toteuttaa saman ja copy-pastetin tämän tiedoston lähes suoraan Joomlan koodista. Sen jälkeen asenteeni on ollut, että kun se kerran toimii niin mitäpä tuota toimivaa muuttamaan.
Olet varmasti oikeassa, että tuo olisi parempi rakentaa uusiksi. Tähän mennessä minulla ei kuitenkaan ole ollut mielenkiintoa lähteä tutkimaan millä tavalla se kannattaisi tehdä vaan olen ennemmin työstänyt tämän projektin muita osia.
Onko ajatukseni oikea, että tämän "viritelmän" voisi toteuttaa paremmin jotenkin näin:
<?php
/**
* Old Factory class
*
* SiteGlobals abstract
*/
abstract class SiteGlobals {
/**
* Global user object
*
* @var User
*/
private static $user;
/**
* Get a user object.
* Returns the global object, only creating it if it doesn't already exist.
*
* @return User object
*/
public static function getUser() {
if (!self::$user)
self::$user = New LUser;
return self::$user;
}
}<?php
/**
* User base object
*/
class LUser {
/**
* THIS WILL BE REMOVED
* Instance container
*
* @var User
*/
private static $instance = null;
public function __construct() {
// Check if user is logged in, get his information and renew session and cookie if needed
$id = $this->loggedInWithSession() ?: $this->loggedInWithCookie();
// If user is not logged in, we set up default public account
$this->userInfo($id);
}
/**
* THIS WILL BE REMOVED
* Returns the global User object, only creating it if it doesn't already exist.
*
* @return User The user object
*/
public static function getInstance() {
if (self::$instance === null) {
self::$instance = new LUser();
}
return self::$instance;
}
// Some other functions
}Noin sitä voi yksinkertaistaa. Toinen vaihtoehto olisi juuri käänteinen, eli erillinen kokoava luokka jätettäisiin pois ja käytettäisiin vain kunkin luokan getInstance-metodia (ehkä selvemmin silloin getGlobalInstance). Ilmeisesti tuossa erillisen luokan idea on tehdä selvemmäksi, mistä asioista on olemassa tällainen globaali kappale.
Yksinkertainen ratkaisu kehäongelmaasi olisi se, että ensin alustaisit olion tyhjäksi ja laittaisit laajemman tietojen haun erilliseen init-funktioon jotenkin tähän tapaan:
<?php
class LUser {
private $name = "tuntematon käyttäjä";
private $id = 0;
public function init() {
$id = $this->loggedInWithSession() ?: $this->loggedInWithCookie();
// jne.
}
}
class SiteGlobals {
public static function getUser() {
if (!self::$user) {
self::$user = new LUser();
self::$user->init();
}
return self::$user;
}
}Vielä pohdinnan arvoinen asia on, miksi edes luodaan olioita, jos niitä on vain yksi per luokka. Olion ideahan on pohjimmiltaan se, että voidaan luoda monta oliota samasta luokasta ja laittaa niihin eri sisältöä. Jos luokka on luonteeltaan sellainen, ettei siitä ikinä tehdä useaa oliota (eli esimerkiksi jos LUser-luokka sisältää tiiviisti istuntoon ja eväisteisiin liittyvää tietoa), olion luominen ei tuo lisäarvoa staattisiin muuttujiin ja metodeihin nähden. Oikeastaan silloin pitäisi estää olion luominen, koska se johtaisi virheisiin: useampi olio muokkaisi samaa globaalia tilaa ja aiheuttaisi ristiriitoja.
Yleensä singletoneilla räpeltäessä tosiaan estetään kokonaan luokan instantiointi muuten kuin singleton-metodin kautta.
final class Foo
{
private static $instance;
private $random;
public static function instance(): Foo
{
if (!static::$instance) {
static::$instance = new static();
}
return static::$instance;
}
private function __construct()
{
/**
* Ei vielä täällä...
*/
// $this->initializeRandom();
}
public function getRandom(): int
{
if ($this->random === null) {
$this->initializeRandom();
}
return $this->random;
}
private function initializeRandom(): void
{
$this->random = rand(100000, 999999);
}
}
var_dump([
Foo::instance()->getRandom(),
Foo::instance()->getRandom(),
]);
/**
* Huom! Instanssin luominen luokan ulkopuolelta käsin ei onnistu.
*/
$foo = new Foo();Metabolix kirjoitti:
Vielä pohdinnan arvoinen asia on, miksi edes luodaan olioita, jos niitä on vain yksi per luokka. Olion ideahan on pohjimmiltaan se, että voidaan luoda monta oliota samasta luokasta ja laittaa niihin eri sisältöä.
Kyllä olioihin liittyy toinenkin periaate: kapselointi.
Tässä tapauksessa tarkoitan kapseloinnilla sitä, että tarvittavat toimenpiteet pitäisi tehdä niissä metodeissa, joita luokan "LUser" käyttäjä kutsuu, eikä luokan omassa konstruktorissa.
Kirjautumistilan alustaminen konstruktorissa on täysin tarpeeton asia, kun sen voi tehdä vasta sitten, kun joku yrittää luokan metodeja kutsumalla tarkistaa, onko käyttäjää autentikoitu tms.
AkeMake kirjoitti:
Mitä tulee tuohon "Factory-viritelmään", olen itsekin pohtinut samaa kysymystä. Mitä hyötyä tuosta on? Neljä vuotta sitten aloitellessani tätä projektia minulla ei ollut mitään käsitystä miten olisin lähtenyt liikkeelle kaikkien näiden luokkien kanssa ja saanut ne toimimaan järkevästi yhteen. Niinpä ilman parempaa tietoa tutkin miten Joomla toteuttaa saman ja copy-pastetin tämän tiedoston lähes suoraan Joomlan koodista.
Yleensä Joomlan kaltaiset projektit ovat käyneet läpi pitkän linjan iteratiivisen kehityskaaren, ja staattisiin singleton-luokkiin on päädytty siitä syystä, että vanha paska koodi on ollut helpointa kääntää sellaiseen muotoon, jotta projekti voi sitten mainostaa käyttävänsä "olio-ohjelmointia".
Singletoneilla voi myös paikata surkeaa riippuvuuksien hallintaa, koska singletonia voi kutsua mielivaltaisesti ihan mistä tahansa ja milloin tahansa, eikä siihen tarvitse saada mitään viitettä, ts. "lupaa käyttää", ulkopuolelta.
Itse en lähtisi koodaamaan mitään vanillana ilman valmiita frameworkeja – en edes opettelumielessä. Se on niin täydellistä ajanhaaskausta, koska siitä ei juurikaan opi mitään eikä oman freimiksen kirjoittamalla voi oikein voittaakaan mitään. Tuhlaat vain aikaa ja se aika on pois itse projektin eli ns. bisneslogiikan kirjoittamiselta.
Kiitos Metabolix ja The Alchemist! Opin vastauksistanne paljon ja sain uutta suuntaa ja pohdittavaa tähän pikku harraste-projektiini liittyen. Erittäin valaisevaa!
Aihe on jo aika vanha, joten et voi enää vastata siihen.