OOP: hoe het object 'Image' definieren?

[Verwijderd]

Hoihoi allemaal,


Ik ben na zeer lange tijd weer aan het programmeren geslaan. Omdat OO mij altijd al enorm heeft geboeid probeer ik mij dan volledig aan de regels ervan te houden. Echter bij het definieren van mijn eerste klasse zit ik eigenlijk al volledig vast. Ik vermoed dat het is omdat het OO-principe er bij mij nog niet helemaal in zit...

OK, genoeg gezeverd, wat is nu juist mijn probleem? Ik wil een online gallery maken (cfr. picasaweb) in PHP met MySQL als achterliggende databank.

Momenteel ziet de constructor van mijn 'Image' klasse als volgt uit:

PHP:

   public function __construct($filename)
   {
      $this->imageInfo=getimagesize($this->filename);
      
      if ($this->imageInfo['mime']=='image/jpeg')
      {
         $this->image=imagecreatefromjpeg($this->filename);
      }
      else if ($this->imageInfo['mime']=='image/gif')
      {
         $this->image=imagecreatefromgif($this->filename);
      }
      else if ($this->imageInfo['mime']=='image/png')
      {
         $this->image=imagecreatefrompng($this->filename);
      }
   }

Dit is uiteraard niet de volledige constructor, maar enkel het gedeelte dat ik denk dat van belang is. Wat ik dust doe is wanneer ik een instantie aanmaak van het object 'Image' gaat de constructor nagaan welk type deze image is vervolgens de informatie en de image zelf in een variabele steken. Dit leek perfect te zijn om allerhande functies om images te bewerken te schrijven (flip, resize, crop, save,...).

Het probleem doet zich echter voor wanneer ik de gegevens over een image uit de databank wil halen of erin wil wegschrijven. Ik kan namelijk geen instantie van het object 'Image' aanmaken aangezien ik op dat moment de filename nog niet weet... Die filenaam ken ik namelijk pas wanneer de gegevens uit de databank gehaald zijn via een functie in de image-klasse.

Ik hoop dat ik mijn probleem een beetje duidelijk heb uitgelegd, ik vind het namelijk enorm moeilijk om het OO-denken in woorden om te zetten.

Voor alle duidelijkheid, ik ben niet op zoek naar PHP-code. Ik zou enkel wat hulp willen bij het bepalen hoe de architectuur van mijn 'Image'-klasse er zou moeten uitzien. Vooral wat ik de constructor van de klasse best kan laten doen?


Alvast dank voor jullie hulp!

[whoami]

Ik zou eens kijken naar het (abstract) factory design pattern.
Je Image class zelf hoeft zich nl. niet bezig te houden met hetgeen jij daar allemaal in die constructor doet.
ik zou een factory maken, die een method 'CreateImage' heeft, daar een filenaam meekrijgt, en die factory gaat dan wel de nodige gegevens uit die file gaan halen, en een 'Image' maken van het juiste type (JpgImage, GifImage, .... die bv allemaal overerven van het basis-type Image).

[NetForce1]

whoami schreef op woensdag 10 december 2008 @ 11:31:
Ik zou eens kijken naar het (abstract) factory design pattern.
Je Image class zelf hoeft zich nl. niet bezig te houden met hetgeen jij daar allemaal in die constructor doet.
ik zou een factory maken, die een method 'CreateImage' heeft, daar een filenaam meekrijgt, en die factory gaat dan wel de nodige gegevens uit die file gaan halen, en een 'Image' maken van het juiste type (JpgImage, GifImage, .... die bv allemaal overerven van het basis-type Image).

Daarbij zou ik het mime-type opslaan in de database zodat je dat mee kan geven aan die factory.

[riddles]

In een andere taal zou je een tweede constructor kunnen maken met daarin een database record id o.i.d. Dat gaat je in PHP helaas niet lukken. Ik zou het inderdaad oplossen zoals whoami zegt, al kan ik me wel voorstellen dat je in een constructor van een image al direct het plaatje in de instantie wilt hebben. Nu moet je extra validatie code bouwen bij aanroep of je instantie wel een plaatje heeft of niet.

[storeman]

Wat Whoami zegt, daarnaast kun je een abstract Image aanmaken en deze vervolgens laten overerven door classen als Image_Png, Image_Jpg of Image_Gif, een extra interface kan hierbij ook wenselijk zijn. Met behulp van een factory wordt er vervolgens een van die objecten aangemaakt en kunnen specifieke open en opslaanfuncties door de subklassen worden uitgevoerd, terwijl algemene bewerkingen in de abstracte klasse gedefineerd staan.

[ppx17]

Voordeel van php is (edit: in dit geval dan..) dat je datatype's niet vast liggen per variabele, daardoor kan je in je constructor ook kijken of je met een array / object (mysql_fetch_array/object) temaken heb. In dat geval kan je verder bouwen op het database record, anders kan je gaan kijken of je de input als filename kan gebruiken. (file_exists). Aan de hand daarvan zou je twee private functies kunnen maken welke je vanuit de echte constructor aanroept. Net welke van de twee je nodig heb.

Beetje workaround, maar doet ook wat jij wilt, zonder extra classes.

[ Voor 3% gewijzigd door ppx17 op 10-12-2008 16:19 ]

[remmelt]

Factory/Controller zeker goede suggestie. Verder zou ik je aanraden om de Image klasse zo generiek mogelijk te houden. Je hebt nu in je constructor een filename die altijd moet bestaan. Stel nou dat je een nieuw Image wilt maken? PHP kan dat, maar jouw klasse heeft altijd een filename en bijbehorend bestand nodig... Misschien wil je je nieuwe Image wel helemaal niet opslaan en alleen over http serven (wegens caching niet aan te raden, maar het is mogelijk) dan heb je dus helemaal geen filename nodig.

Wat je nogal eens ziet bij PHP is dat er functies zijn zonder benoemde parameters, en dat de parameters dan middels func_get_args() worden uitgelezen. Dat is zeker een optie die je kan gebruiken, maar mooi is het niet. Niks aan te doen, PHP kan niet anders.

[DutchCommando]

ppx17 schreef op woensdag 10 december 2008 @ 16:19:
Voordeel van php is (edit: in dit geval dan..) dat je datatype's niet vast liggen per variabele, daardoor kan je in je constructor ook kijken of je met een array / object (mysql_fetch_array/object) temaken heb. In dat geval kan je verder bouwen op het database record, anders kan je gaan kijken of je de input als filename kan gebruiken. (file_exists). Aan de hand daarvan zou je twee private functies kunnen maken welke je vanuit de echte constructor aanroept. Net welke van de twee je nodig heb.

Beetje workaround, maar doet ook wat jij wilt, zonder extra classes.

Jou suggestie is dus om workaround (hacking) toe te passen om het aantal classes maar klein te houden? Zonde, op deze manier maak je het jezelf vaak onnodig lastig. De Image klasse heeft in jouw oplossing veel teveel kennis (en verantwoordelijkheid) van andere delen van het systeem. Jouw oplossing geeft de Image klasse niet alleen de verantwoordelijkheid een Image te 'bewaren' inclusief bijbehorende metadata. Neen, jouw oplossing gaat verder door:

1. teveel verantwoordelijkheid bij de constructor te leggen (controle van invoer type (zelfs directe kennis van mysql?) en doorvoer naar verschillende methodes). Dit gedeelte moet zoals gezegd verplaatst worden naar een factory.
2. I/O operaties uit te voeren om een naam te controleren. (Hoort totaal niet in image thuis).


Naast de genoemde suggesties m.b.t. overerving blijkt misschien dat het nodig is om dezelfde opslag implementatie, of delen hiervan, meerdere malen te gebruiken. In dat geval wordt het strategy pattern hier ook toepasbaar.

[Flard]

Ik ben het grotendeels eens met whoami, maar ik vraag me af wat de meerwaarde is van een aparte JpgImage en PngImage die erven van (Base)Image. Mochten die inderdaad aparte implementaties hebben zou het nuttig zijn. Maar het énige verschil is volgens mij de manier waarop ze aangemaakt worden (imagecreatefromjpeg vs imagecreatefrompng), voor de rest kun je er daarna hetzelfde mee. (ook kun je een image die ooit van een .jpg kwam ook gerust opslaan naar een .png, etc.)

Tenzij het verschil in bestandsformaat echt belangrijk is (ik kan me ergens voorstel dat je in PngImage misschien iets met alphatransparancy zou willen doen), zou ik eerder kiezen voor een ImageFactory, die op basis van het mime-type (of een db-record, al zie ik de TS nergens over een database praten) een Image-instantie teruggeeft. (Je Image zou als constructor dan een image resource-handle kunnen ($im) accepteren).

[ppx17]

DutchCommando schreef op woensdag 10 december 2008 @ 17:27:
[...]


Jou suggestie is dus om workaround (hacking) toe te passen om het aantal classes maar klein te houden? Zonde, op deze manier maak je het jezelf vaak onnodig lastig. De Image klasse heeft in jouw oplossing veel teveel kennis (en verantwoordelijkheid) van andere delen van het systeem. Jouw oplossing geeft de Image klasse niet alleen de verantwoordelijkheid een Image te 'bewaren' inclusief bijbehorende metadata. Neen, jouw oplossing gaat verder door:

1. teveel verantwoordelijkheid bij de constructor te leggen (controle van invoer type (zelfs directe kennis van mysql?) en doorvoer naar verschillende methodes). Dit gedeelte moet zoals gezegd verplaatst worden naar een factory.
2. I/O operaties uit te voeren om een naam te controleren. (Hoort totaal niet in image thuis).


Naast de genoemde suggesties m.b.t. overerving blijkt misschien dat het nodig is om dezelfde opslag implementatie, of delen hiervan, meerdere malen te gebruiken. In dat geval wordt het strategy pattern hier ook toepasbaar.

Hoeft uiteraard niet letterlijk genomen te worden, als je volledig OO schrijft kan je invoer ook een instanceof een ander object te zijn. Bijvoorbeeld een File en DbRecord object oid. Dan hoeft je Image klasse behalve het bestaan van deze objecten weinig meer te weten en kunnen ook de methoden voor opslaan ed. via die objecten worden uitgevoerd. Alles exact 1 klasse opschuiven naar de factory zie ik ook nog niet heel erg zitten. Maar misschien kijk ik ergens wel heel erg overheen?

Om even naar C# te kijken, daar kan je met de Bitmap : Image klasse ook direct bestanden uit memory (db?) en filesystem lezen en schrijven, dus zo'n hele rare gedachte is het toch niet?

[whoami]

ppx17 schreef op woensdag 10 december 2008 @ 23:07:
[...]

Om even naar C# te kijken, daar kan je met de Bitmap : Image klasse ook direct bestanden uit memory (db?) en filesystem lezen en schrijven, dus zo'n hele rare gedachte is het toch niet?

Dat gaat daar wel even anders.
Die method die een string als argument neemt (filename), zal de call gewoon delegeren naar die andere method die een Stream als argument neemt.
Het ene wat die eerste method (die een string als argument heeft), eerst zal doen, is een Filestream object instantieren, en dan de andere overload aanroepen.
Dat is wel ietsje cleaner denk ik dan hetgeen jij beschrijft.

Alles exact 1 klasse opschuiven naar de factory zie ik ook nog niet heel erg zitten. Maar misschien kijk ik ergens wel heel erg overheen?

Het is niet het 'exact verschuiven naar één class'.
het is het weghalen van verantwoordelijkheden.
Als het instantieren van een Image een complexe taak is (van waar haal ik de gegevens, over welk soort Image gaat het , etc... ) dan hoort deze verantwoordelijkheid niet thuis bij Image, maar in een factory.
Zeker als het concrete type dat moet geinstantieerd worden, afhankelijk is van bepaalde gegevens.

[ Voor 26% gewijzigd door whoami op 10-12-2008 23:30 ]

[PrisonerOfPain]

In PHP zou ik het ongeveer zo aan pakken:

PHP:

interface image_file{
    function load($file_name);
    function store(canvas $canvas, $file_name);
}

class image_file_bmp implements image_file {} // et cetera

class canvas_factory{
    function register(image_file $file, $file_ext){ /* store $file and $file_ext in a map somewhere */ }

    function load($filename) { /* use the correct image_file class and return a canvas based on filename */ }
    function store(canvas $canvas, $file_name){ /* find correct image_file and store $canvas */ }
}

class canvas{
    public function circle($x, $y, $radius) { /* draw a circle */ }
    // et cetera

    $pixels = array();
}

image_file neemt gewoon een string als file name, en leest 'm eventueel handmatig uit. Dit heeft als voordeel dat de eind gebruiker gewoon een stream wrapper kan schrijven als 'ie z'n bestanden uit de database wil halen; the stream API van PHP is best flexibel maar ook behoorlijk transparant dus wellicht is het een goed idee om dit te documenteren.

canvas_factory is verantwoordelijk voor het bijhouden van de image_file classes en het selecteren van de juiste. Momenteel word er gebruik gemaakt van de bestands extensie alleen kan dat problemen op leveren niet flexible genoeg (database stream hoeft geen extensie te hebben), geen verplichte mime-type checking wat kan leiden tot beveiligings lekken. Als dat een probleem is zou ik gewoon een method aan image_file toevoegen om te kijken of 'ie 'm uberhaupt kan inladen. Dan kan de canvas_factory door de lijst lopen en de goede selecteren.

canvas is gewoon je pixel buffer waar je onafhankelijk van 't bestandsformaat heen kan schrijven.

Dit is een scheiding op de manier die ik destijds had gemaakt omdat ik ook bestanden wou kunnen openen die GD2 niet kon lezen (tga, bmp, tiff et cetera) ook heeft het als voordeel dat alle GD2 specifieke buiten de canvas class om gebeurd, mocht ik op image magick over willen gaan, of alles handmatig doen kan dat ook.

Uiteindelijk is het design nog iets anders geworden als hier geschetst, zo is canvas_factory een singleton geworden omdat ik 'm dan makkelijk aan kon roepen vanuit de constructor van canvas; het doel is uiteindelijk om een duidelijke en transparante API te maken die makkelijk in gebruik is voor het algemene geval en het schrijven van een file loader is geen algemeen geval. Dus dat word voor de gemiddelde gebruiker verstopt.

[frickY]

Is het hier niet logisch static-methods te gebruiken welke een instantie van je class terug geven?

PHP:

<?php
class Image {
    protected $image;
    protected __construct() {
        
    }
    
    public static loadFromFile($filename) {
        return self::loadFromString(file_get_contents($filename));
        
    }
    
    public static loadFromString($imagedata) {
        $instance = new self;
        $instance->image = $imagedata;
        return $instance;
    }
}

$image = Image::loadFromFile('foo.gif');
$image2 = Image::loadFromStrubg($row['columnImage']);
?>

[ Voor 4% gewijzigd door frickY op 11-12-2008 12:09 ]

[PrisonerOfPain]

frickY schreef op donderdag 11 december 2008 @ 12:06:
Is het hier niet logisch static-methods te gebruiken welke een instantie van je class terug geven?

De abstractie die je hier probeert te maken word al gedaan door PHP's eigen stream api