[PHP] Sub class aanroepen in class

[Verwijderd]

Beste leden,

Ik ben bezig met het opzetten van een API voor onze systemen. Hiervoor hanteren we de volgende structuur:

http://api.site.nl/comman...ommand=add&parameters....

Nu wil ik in de command.php de auth uitvoeren en overige dingen controleren of een command en subcommand mogelijk zijn waarna die verder moet gaan in een andere class om het command te verwerken.

Nu ben ik begonnen met het opzetten van de command class (zie onderstaand):

PHP:

<?php
error_reporting(E_ALL);

/**
 * Class command
 * 
 * 
 */

class Command {

    /**
     * User loggedIn
     * 
     * @acces private
     * @var integer 
     */

    private $loggedIn = NULL;

    /**
     * UserId of the user that has loggedin to the API
     * 
     * @acces private
     * @var integer 
     */

    private $userId;

    /**
     * Database connection
     * 
     * @acces private
     * @var obj 
     */

    private $oDB;

    /**
     * Allowed commands 
     * 
     * @acces private
     * @var array 
     */

    private $allowedCommands = array('ADD', 'DEL', 'UPD', 'LIST', 'GET', 'UPGR');

    /**
     * Command 
     * 
     * @acces private
     * @var array 
     */

    private $command;


    /**
     * the __construct for alle the commands
     * 
     * @acces public
     */

    public function __construct() {
 
        //connect with the database

        $oDB = new mysqli('', '', '', '');

        $this->oDB = $oDB;

        //user data check

        $this->auth($_GET['apiuser'], $_GET['apipass']);

        //user loggedin correct?

        if($this->loggedIn == 1) {

            //check if the command is allowed

            $this->validateCommand($_GET['command']);

        } else {

            throw new Exception('Login data is incorrect!'); 

        }
        
        echo $this->command;

    } 

    /**
     * auth function to check the user
     * 
     * @acces private
     * @var user string  
     * @var pass string  
     */

    private function auth($user, $pass) {

        //check the user and pass

        $getUser = $this->oDB->query("
            SELECT 
                id
            FROM 
                users
            WHERE 
                username = '" . mysqli_real_escape_string($user) . "'
            AND
                pass = '" . mysqli_real_escape_string(md5($pass)) . "'
        ") or die (mysqli_error($this->oDB));

        //checks if the userdata is correct

        if($getUser->num_rows == 1) {
            
            $listUser = $getUser->fetch_array();

            $this->loggedIn = 1;
            $this->userId = $listUser['id'];

        } else {

            $this->loggedIn = NULL;
            $this->userId = NULL;

        }

    }

    /**
     * validate the command
     * 
     * @acces private
     * @var method string
     */

    private function validateCommand($method) {
        
        //check if command is allowed
        
        if(in_array(strtoupper($method), $this->allowedCommands)) {
        
            $this->command = $method;
        
        } else {

            echo 2;

        }
        
    }

    /**
     * setup the sub command
     * 
     * @acces private
     */

    private function Command() {

    }

}

$a = new Command();

Nu heb ik twee vragen:
- hebben jullie nog opmerkingen mbt bovenstaande opzet;
- hoe kan ik de command functie het beste aanpakken, vanaf hier zal de class opgeroepen moeten worden om bijv. een email adres toe te voegen. Gezien er vele verschillende soorten commando's kunnen zijn zoek ik een zo universele oplossing mogelijk.

Piete

[Voutloos]

Dus niet idempotente acties gaan gewoon via GET?

Zoek dat keyword maar op, inclusief de definitie van de verschillende HTTP methods.

[Woy]

PHP:

mysqli_real_escape_string(md5($pass))

Dat is nogal overkill , je weet immers wat voor output er uit de md5 functie komt, en dus weet je dat daar geen special characters in kunnen zitten.

[Verwijderd]

Voutloos schreef op maandag 08 februari 2010 @ 10:23:
Dus niet idempotente acties gaan gewoon via GET?

Zoek dat keyword maar op, inclusief de definitie van de verschillende HTTP methods.

Met HTTP methode denk ik dat je wil zeggen, kijk eens naar een REST api method enz? Dit is geen optie voor ons. Men wil dat alle API commando's direct in de adres balk uit te voeren zijn als api command.

En je opmerking mbt de GET bedoel je neem ik aan dat ik de de vars mee moet geven aan de constructor? Indien correct, hoe hier dan mee omgaan aangezien de commando's verschillende parameters kunnen hebben.

Woy schreef op maandag 08 februari 2010 @ 10:28:
[...]

Dat is nogal overkill , je weet immers wat voor output er uit de md5 functie komt, en dus weet je dat daar geen special characters in kunnen zitten.

Daar heb je gelijk in, had het er net even snel ingezet om daar geen opmerkingen over te krijgen dat de GET niet gecontroleerd wordt.

Overigens ter informatie dit wordt mijn eerste serieuze class, dus er kunnen we eens vreemde dingen in zitten .

[ Voor 28% gewijzigd door Verwijderd op 08-02-2010 10:35 ]

[Voutloos]

Verwijderd schreef op maandag 08 februari 2010 @ 10:33:
Men wil dat alle API commando's direct in de adres balk uit te voeren zijn als api command.

Dan moet je juist je leestaakje uitvoeren en vervolgens hun in een uitgebreid betoog melden dat dit een slecht idee is.

[Janoz]

Verwijderd schreef op maandag 08 februari 2010 @ 10:33:
[...]

Met HTTP methode denk ik dat je wil zeggen, kijk eens naar een REST api method enz?

Nee, daarmee wordt bedoelt: "Lees hoofdstuk 9 van de http specs even door". Heb daarbij speciale aandacht voor 9.1.2.

[Verwijderd]

Heb het eens doorgelezen, verbeter me als ik fout zit, maar het komt er volgens mij op neer dat de GET geen consequenties mag hebben, maar enkel data ophalen en dat je met de HTTP methodes PUT, POST en DELETE data mag aanpassen.

[Janoz]

Klopt. De GET mag in principe de state van de server niet veranderen. Het zou niet uit moeten maken of je hetzelfde request 1x of meerdere keren doet.

[MueR]

Is het overigens wel een strak idee om username en password mee te geven in de API? Ik ga er voor het gemak van uit dat men met die gegevens ook ergens kan inloggen namelijk. Is een API key niet een betere oplossing voor authenticatie?

[jbdeiman]

username/ password meegeven is niet handig, beter is het om een (evt. username) maar een unieke hash te genereren die dan meegegeven wordt aan de url. Deze hash wordt aangemaakt voor iemand en is per persoon uniek. => Het password is dan niet te achterhalen/ aan te passen wanneer je via de api bezig bent.
Zo voorkom je diefstal van een account.

[Verwijderd]

Ik ga zeker de gegeven kritiek tot me nemen en bekijken wat ik er mee ga doen (en kan doen). Maar mijn hoofd vraag was, wat is de beste methode, wanneer de validatie van de user akkoord is, verder te gaan naar de class die bijvoorbeeld de actie verwerkt voor een email adres?

[ Voor 34% gewijzigd door Verwijderd op 08-02-2010 13:36 ]

[Janoz]

om die vraag te beantwoorden zul je even een stapje terug moeten doen. OOP technisch ben je namelijk de verkeerde kant op aan het redeneren. Vanuit een klasse kun je nooit naar de afgeleide klasse gaan omdat de parent klasse helemaal niks hiervan weet. In OOP werkt het juist de andere kant op. De afgeleide klasse is juist degene die naar de parentklasse refereert. Je hebt een emailVerwerkCommand klasse. In een 'echte' OO taal zou je bijvoorbeeld iets als een abstracte doCommandAction methode maken die de afgeleide klasse moet implementeren. Hoe en of dit in php kan zou ik niet weten.

[chris]

Je kan het mooi oplossen door het in twee stukken te verdelen: een abstracte command class, met bijvoorbeeld validateCommand en executeCommand. De subclasses implementeren en overriden waar nodig.

Vervolgens kan je een aparte function "createCommand" maken, die een string omzet naar de juiste klasse van een Command. Je zou ook wellicht het factory pattern kunnen bestuderen, of de routing van ruby on rails. Ik zou dit persoonlijk loskoppelen van de logica van die classes.

[Verwijderd]

Woy schreef op maandag 08 februari 2010 @ 10:28:
[...]

Dat is nogal overkill , je weet immers wat voor output er uit de md5 functie komt, en dus weet je dat daar geen special characters in kunnen zitten.

Het is inderdaad overkill, maar niet fout. Sterker nog ik zie dit liever dan andersom waarbij er niets wordt ge-escaped. Eigen zou je ook nog een DAL willen hebben. maar goed mierenneuken in de marge en gaat totaal niet in op de vraag van OT.

Janoz schreef op maandag 08 februari 2010 @ 11:41:
Klopt. De GET mag in principe de state van de server niet veranderen. Het zou niet uit moeten maken of je hetzelfde request 1x of meerdere keren doet.

Helaas wordt dit maar zelden goed toegepast

[ Voor 23% gewijzigd door Verwijderd op 08-02-2010 20:08 . Reden: betere edit dan dubbelpost ;) ]

[Janoz]

"Zelden" zou ik absoluut niet willen zeggen. Wat wel zo is, is dat er veel 'prutsers' rondlopen die eigenlijk niet niet genoeg van wanten weten en dan vervolgens gaan klagen op de spiderbot van google die allemaal acties uithaalt die ze niet willen (of dat ze neit worden geindexeerd omdat hun javascript menu met post requests werkt).

Al moet ik toegeven dat ik zelf ook wel eens gezondigd heb omdat ik een minimalistische implementatie nodig had waarbij ik nog makkelijk het verschil tussen een normaal en een inlog request nodig had..

[Verwijderd]

chris schreef op maandag 08 februari 2010 @ 16:46:
Je kan het mooi oplossen door het in twee stukken te verdelen: een abstracte command class, met bijvoorbeeld validateCommand en executeCommand. De subclasses implementeren en overriden waar nodig.

Heb me een beetje ingelezen mbt het abstract class gedeelte en wat ik eruit opmaak is dat je hierin aangeeft welke functies een child class minimaal moet bezitten.

Je zegt de subclasses implenteren en overriden, met implenteren bedoel je neem ik aan het extenden van de abstracte command class. Wat ik nu nog niet goed inzien, waar komt de user nu binnen en wat is dan het verloop van het command dat de user wil uitvoeren?

Edit:
Kom net de functie implements tegen op php.net. Dit ga ik nu even doorlezen.

[ Voor 5% gewijzigd door Verwijderd op 08-02-2010 21:42 ]

[Verwijderd]

Na wat google kwam ik een voorbeeld tegen welke naar mijn idee is wat Chris bedoelt:

PHP:

<?php
 
abstract class Pizza
{
    protected $_price;
    public function getPrice()
    {
        return $this->_price;
    }
}
 
class HamAndMushroomPizza extends Pizza
{
    protected $_price = 8.5;
}
 
class DeluxePizza extends Pizza
{
    protected $_price = 10.5;
}
 
class PizzaFactory
{
    public static function createPizza($type)
    {
        $baseClass = 'Pizza';
        $targetClass = ucfirst($type).$baseClass;
 
        if (class_exists($targetClass) && is_subclass_of($targetClass, $baseClass))
            return new $targetClass;
        else
            throw new Exception("The pizza type '$type' is not recognized.");
    }
}
 
$pizzas = array('HamAndMushroom','Deluxe','Hawaiian');
foreach($pizzas as $p) {
    printf(
        "Price of %s is %01.2f".PHP_EOL ,
        $p ,
        PizzaFactory::createPizza($p)->getPrice()
    );
}

Nu zit ik met nog met een paar vraagjes:
- de validatie van de user gegevens, komt deze dan in de abstract class Pizza?
- waarom is de createPizza functie in de PizzaFactory static?

[Janoz]

het aangehaalde voorbeeldje is eigenlijk een beetje een ruk voorbeeld omdat het 2 verschillende dingen laat zien terwijl jij je op dit moment eigenlijk maar op 1 van die dingen wilt richten. Eigenlijk moet je op dit moment even volledig de Factory class die ze daar hebben negeren (sowieso vind ik die erg lelijk met al die variabele variabelen). Als je dit vervolgens weglaat dan zijn de extended voorbeelden wel redelijk karig, aangezien ze enklel een variabele aanpassen.

In jou situatie zul je meer hebben aan een voorbeeld als dit:
(is allemaal uit het hoofdje dus ik heb geen idee of het uberhaupt compileert)

PHP:

abstract class Command {
  public function doCommand($someArgs) {
    //validate and stuff
    return doSpecificCommand($someArgs);
  } 
} 
  
class EmailUpdateCommand extends Command {
  public function doSpecificCommand($someArgs) {
    //do email stuff
    return $someResult;
  }
}


$command = new EmaiLUpdateCommand();
$command->doCommand($someArgs);

[Verwijderd]

Bedankt voor je duidelijk uitleg. maar gezien we te maken hebben met niet enkel het email command zullen we toch op een flexibele manier andere classes moete kunnen aanroepen waardoor het voorbeeld wat ik gaf me wel aanspreek.

Mij lijkt dit gedeelte dan toch wel van noodzaak:

PHP:

<?php
class PizzaFactory
{
    public static function createPizza($type)
    {
        $baseClass = 'Pizza';
        $targetClass = ucfirst($type).$baseClass;
 
        if (class_exists($targetClass) && is_subclass_of($targetClass, $baseClass))
            return new $targetClass;
        else
            throw new Exception("The pizza type '$type' is not recognized.");
    }
} 
?>

[Janoz]

Welk probleem denk je daarmee op te lossen?

imho is

PHP:

$pizza = PizzaFactory->createPizza("DeluxePizza");

een onnodig omslachtige manier om

PHP:

$pizza = new DeluxePizza ();

te doen. Op mij komt het heel erg over als het cargo cult antipattern. Factories hebben zeker hun nut, maar dat nut komt pas om de hoek kijken wanneer ze daadwerkelijk een probleem op moeten gaan lossen.

Het hebben van verschillende afleidingen (extended classen zeg maar) van dezelfde (abstracte) class is zeker nog geen reden voor het hebben van een factory.

[Verwijderd]

Janoz schreef op woensdag 10 februari 2010 @ 14:10:
Welk probleem denk je daarmee op te lossen?

Het probleem van het ophalen van de juiste (extended) class bij de juiste command.

[Janoz]

Heb je daaru uberhaupt een probleem? Waarom werkt new niet? Wat is exact de reden waarom new niet werkt en er dus een factory nodig is?

[Verwijderd]

Janoz schreef op woensdag 10 februari 2010 @ 14:35:
Heb je daaru uberhaupt een probleem? Waarom werkt new niet? Wat is exact de reden waarom new niet werkt en er dus een factory nodig is?

Ik denk omdat ergens op een bepaald punt de incomming command (string) omgezet moet worden naar een Class die bij die command hoort.

Zelf zou ik er dan gewoon een lijstje van maken:

PHP:

$_concreteCommandPool = array(
  'NEW' => 'Command_New',
  'ADD' => 'Command_Add'
  // etc
)

$command = new $_concreteCommandPool[$incomming];

Volgens mij staat hier ook precies beschreven wat de TS wil:
Wikipedia: Command pattern

[Janoz]

Ik ken het command pattern en ik begrijp wat de topicstarter wil doen, maar ook in het aangehaalde wiki artikel komt nergens het woord factory terug. Neem nu je eigen code. Daar is het gebruik van de factory eigenlijk complete onzin. Waarom eerst iets naar een string doen om vervolgens daar weer middels variabele variabelen een class van maken?

Neem je eigen voorbeeld:

PHP:

<?php 
$_concreteCommandPool = array( 
  'NEW' =>new NewCommand(), 
  'ADD' => new AddCommand()
  // etc 
) 

$command = $_concreteCommandPool[$incomming];

[Verwijderd]

Janoz schreef op woensdag 10 februari 2010 @ 15:17:
Ik ken het command pattern en ik begrijp wat de topicstarter wil doen, maar ook in het aangehaalde wiki artikel komt nergens het woord factory terug. Neem nu je eigen code. Daar is het gebruik van de factory eigenlijk complete onzin. Waarom eerst iets naar een string doen om vervolgens daar weer middels variabele variabelen een class van maken?

Neem je eigen voorbeeld:

PHP:

<?php 
$_concreteCommandPool = array( 
  'NEW' =>new NewCommand(), 
  'ADD' => new AddCommand()
  // etc 
) 

$command = $_concreteCommandPool[$incomming];

Een factory zou handig kunnen zijn als je vanuit meerdere plekken commands wil kunnen aanmaken. Of zou je dan overal zorgen dat de lijst met beschikbare commands en hun classes beschikbaar is?

Desnoods doe je het zo:

PHP:

<?php
class CommandFactory {
  private static $_concreteCommandPool = array( 
    'NEW' =>new NewCommand(), 
    'ADD' => new AddCommand()
    // etc 
  );
  
  public static function createCommand($incomming) {
    return self::$_concreteCommandPool[$incomming]; 
  }

  public static function getAvailableCommands() {
    return array_keys(self::$_concreteCommandPool);
  }
}
?>

Maar je zou de factory dan ook kunnen automatiseren dat hij zelf de classname aanmaakt aan de hand van de string van de incomming command (en kijkt of die Class een implementatie is van het Abstract Command). Dan hoef je niet iedere keer als je een nieuwe command implementeerd de lijst aan te passen. En dan kom je uit op het voorbeeld van de Pizzafactory hierboven.

Edit: Nu ik er trouwens over nadenk, is dit voorbeeld hierboven eigenlijk helemaal niet goed. Iedere keer als er een command aangemaakt wordt zullen er instanties van alle commands aangemaakt worden (niet goed voor het geheugen gebruik), maar wat erger is is dat als je bijvoorbeeld 2x een AddCommand wil aanmaken, dat dan dezelfde instantie teruggegeven wordt in plaats van iedere keer een nieuwe instance van AddCommand. Eigenlijk zijn alle commands hiermee dus 'singleton' geworden.
Ik zou echt gewoon
$command = new $_concreteCommandPool[$incomming];
gebruiken, zodat iedere keer een nieuwe instance wordt aangemaakt en er niet onnodig instance van alle commands aangemaakt worden. Ondanks dat het er misschien lelijk uitziet...

[ Voor 18% gewijzigd door Verwijderd op 10-02-2010 16:00 ]

[YopY]

Verwijderd schreef op woensdag 10 februari 2010 @ 15:27:
Edit: Nu ik er trouwens over nadenk, is dit voorbeeld hierboven eigenlijk helemaal niet goed. Iedere keer als er een command aangemaakt wordt zullen er instanties van alle commands aangemaakt worden (niet goed voor het geheugen gebruik), maar wat erger is is dat als je bijvoorbeeld 2x een AddCommand wil aanmaken, dat dan dezelfde instantie teruggegeven wordt in plaats van iedere keer een nieuwe instance van AddCommand.

Lijkt me niet zo'n moeilijk probleem:

PHP:

function createCommand($name) {
  switch($name) {
    case 'ADD': return new AddCommand();
    case 'DEL': return new DelCommand();
    default: throw new InvalidCommandException($name);
  }
}

Dat is niet eens zoveel moeilijker dan 'dynamisch' (aan de hand van een stringwaarde) een object aanmaken, en als je een nieuw command aanmaakt voeg je die gewoon toe aan de lijst.

De eerder aangedragen oplossing (met een array van van te voren geinitialiseerde command objecten) is meer iets voor een stateful applicatie (Java / C# etc).

En het tweemaal aanmaken van een commando lijkt me in dit geval niet van toepassing - daar je per API call toch maar één keer een commando aan kunt maken. Als je dit in een stateful webapp gebruikt is dat echter wel van toepassing, maar dan kun je ook nadenken over je eigenlijke commands, dwz of die wel een state moeten hebben afhankelijk van de call. Zelf heb ik ook eens zoiets in elkaar geschroefd (alhoewel ik het RequestHandlers noemde), die gewoon éénmaal aangemaakt werden en verder, afgezien van een aantal services, stateless waren. Die ontvingen een request, haalden iets uit de database, en gaven het resultaat terug. Geen state nodig, in zo'n geval.

[Janoz]

Verwijderd schreef op woensdag 10 februari 2010 @ 15:27:
[...]

Een factory zou handig kunnen zijn als je vanuit meerdere plekken commands wil kunnen aanmaken. Of zou je dan overal zorgen dat de lijst met beschikbare commands en hun classes beschikbaar is?

Kijk, maar dan heb je een probleem dat je moet oplossen.

Desnoods doe je het zo:

PHP:

<?php
class CommandFactory {
  private static $_concreteCommandPool = array( 
    'NEW' =>new NewCommand(), 
    'ADD' => new AddCommand()
    // etc 
  );
  
  public static function createCommand($incomming) {
    return self::$_concreteCommandPool[$incomming]; 
  }

  public static function getAvailableCommands() {
    return array_keys(self::$_concreteCommandPool);
  }
}
?>

Dat is alvast een stuk nettere implementatie van een factory.

Maar je zou de factory dan ook kunnen automatiseren dat hij zelf de classname aanmaakt aan de hand van de string van de incomming command (en kijkt of die Class een implementatie is van het Abstract Command). Dan hoef je niet iedere keer als je een nieuwe command implementeerd de lijst aan te passen. En dan kom je uit op het voorbeeld van de Pizzafactory hierboven.

Waarom moet je automatiseren? Je weet toch wat voor applicatie je maakt en welke commands je gaat gebruiken? Waarom is het zo'n probleem om een lijst bij te houden van je commands? Dergelijke reflectie en variabele variabelen oplossingen vind ik persoonlijk erg lelijk. Het zijn dingen die in een erg groot generiek framework thuishoren in situaties waarbij je bij oplevering van je product nog niet weet welke commands er uiteindelijk gebruikt gaan worden.

[Makkelijk]

Ik ga er een beetje vanuit dat je het Command pattern wilt implementeren als ik je woordkeuze zie. Wat je nu hebt gemaakt is infeite de Invoker dus ik snap niet echt dat je die Command noemt.
Maak voor elk commando een klasse met een execute methode. In execute ga je daadwerkelijk dingen doen. De invoker controleert zelf of het commando bestaat en roept de juiste command aan, dat ben je nu in feite aan het doen. Wat is je argumentatie om de klassenaam te baseren op het commando? Als je een nieuw commando maakt scheelt het maar op 1 plaats aanpassen. Is het niet veel makkelijker een switch te gebruiken?

Mocht je een wat groter framework maken dan ik nu beweer en bijvoorbeeld voor 20 verschillende modules 5 verschillende commands gaan maken (100 klassen dus) dan zit het anders. Dan zou ik die 5 commando's definiëren zoals eerder gezegd met switch en alleen de modulenaam dynamisch gebruiken in het aanroepen van het commando.

[_eXistenZ_]

Janoz schreef op woensdag 10 februari 2010 @ 16:16:
[...]

Kijk, maar dan heb je een probleem dat je moet oplossen.


[...]

Dat is alvast een stuk nettere implementatie van een factory.

http://php.net/manual/en/function.call-user-func-array.php kan toch ongeveer hetzelfde?

[ Voor 4% gewijzigd door _eXistenZ_ op 11-02-2010 01:18 . Reden: Zodat de devvers zich niet verdrietig voelen om ongebruikte functionaliteit ]

[Janoz]

_eXistenZ_ schreef op donderdag 11 februari 2010 @ 01:17:
[...]


http://php.net/manual/en/function.call-user-func-array.php kan toch ongeveer hetzelfde?

Ik begrijp niet precies wat je hier mee bedoeld.

[Verwijderd]

Makkelijk schreef op donderdag 11 februari 2010 @ 00:06:
Ik ga er een beetje vanuit dat je het Command pattern wilt implementeren als ik je woordkeuze zie. Wat je nu hebt gemaakt is infeite de Invoker dus ik snap niet echt dat je die Command noemt.
Maak voor elk commando een klasse met een execute methode. In execute ga je daadwerkelijk dingen doen. De invoker controleert zelf of het commando bestaat en roept de juiste command aan, dat ben je nu in feite aan het doen. Wat is je argumentatie om de klassenaam te baseren op het commando? Als je een nieuw commando maakt scheelt het maar op 1 plaats aanpassen. Is het niet veel makkelijker een switch te gebruiken?

Command patern komt, naar mijn idee, het meest in de buurt van onze situatie. Ga hier nog eens wat meer informatie over opzoeken om tot een goede structuur te komen. Wat voor benaming wilde jij deze classes dan gaan geven als je hem niet de naam van het desbetreffende command gaat geven.