Compass : East Oriented

[srm]

Note d'abord que, pour les deux cas, c'est hyper-verbeux alors que le cas n'est pas si complexe (tu trouves pas?)

Oui East et pas mal verbeux.

• J'ai demandé "si j'implémente Kid et Women", pas "Si j'implémente WomenKid". Dans ta structure, une classe externe qui implémenterait Women et Kid serait rejetée car elle n'implément pas WomenKid. Pourtant, elle implément Women et Kid, comme je l'ai demandé. Les specs ne sont pas respectées.

Oui mais une limitation dans PHP ne permet pas de le faire proprement, le plus propre et donc d'avoir un interface composé.

• Si ma classe implémente Women+Kid, et qu'elle appelle askWomenByGold, pour moi, elle devrait recevoir l'argent d'une Women+Kid. Dans ton code, la classe appelante doit savoir qu'il y a une séparation de cas entre Kid, Women, et KidWomen.

Oui et c'est normal c'est elle qui doit choisir comme se présenter, j'ai donné le cas d'un Policier en Civil.
C'est à lui de se présenter comme un Policier ou un Civil.

• Je trouve dommage de stocker le $gold->getValue() dans Conan. Pourquoi pas le $gold directement?

Avoir un valueObject permet de laisser la responsabilité au valueObject de ce qui définie si la valeur et bonne pour le valueObject Gold ou non. De plus ça permet de s'assurer que lorsque l'on me donne de l'or on ne me le donne pas sous n'importe quel format.

• Si t'es forcé de passer par un WomenKid pour faire du Women+Kid, tu ne sens pas l'explosion combinatoire qui arrive? il faudra implémenter les interfaces AB pour chaque couple utile? ABC pour les triplets? etc? Et si tu crées ABC, il faudra l'implémenter dans tous les codes dont la classe est A,B,C ? Rien ne va te le signaler.

Encore une fois, c'est du à une limitation de PHP, c'est pas de ma faute à moi
Et en effet ça pose problème, il va falloir attendre que PHP ajoute ça.
Si je créé ABC ca ne vas pas du tout casser une classe qui implémente A,B,C car quand elle va fait askSomethingByABC() ça ne marchera pas car elle ne respectera pas l'interface, comme d'habitude donc.

• Heu, c'est pas du East: j'ai return $this->value; dans Age

En East le postulat "on ne retourne jamais autre chose que this" et pour simplifié à quelqu'un qui débute en East.
Toutes les classes qui ont un comportement et qui donc réagisse à l'environnement (qui sont donc des classes qui implémentent forcément une interface) doivent retourner this sur leurs méthodes publiques. Un valueObject n'a aucun comportement ou ne sais pas réagir à son environnement, il ne fait que transporter une donnée.

• T'as pas l'impression que executeIfGreaterOrEqualTo sera copié-collé dans d'autres classes (comme Gold) ?

Tous les valueObject n'ont pas besoin d'être comparer, mais pour ceux pour qui c'est le cas tu peux parfaitement faire une abstract NumberComparison

• ageOfWomenIs peut être appelé dans l'ordre qu'on veut:

Code PHP :

<?php 

class PetitePeste implements Women {

   public function ageIsAskedByHero(Hero $hero) {

   }

   public function racketer(Hero $hero) {

       // Ca pourrait venir d'un autre bout de code:

       // Dans un tel cas, bon courage pour débugger :)

       $hero->ageOfWomenIs(new Age(42));



       $hero->askGoldByWomen($this);

   }

} 

[srm]

Je me suis un peu planté pour l'or infini, voilà la correction.

<?php



interface GiveGoldByHero

{

    public function giveGoldByHero(Gold $gold, Hero $hero);

}



interface Kid extends GiveGoldByHero { }



interface Women extends GiveGoldByHero

{

    public function ageIsAskedByHero(Hero $hero);

}



interface WomenKid extends Women, Kid { }



interface Hero

{

    public function askGoldByWomen(Women $women);

    public function askGoldByKid(Kid $kid);

    public function askGoldByWomenKid(WomenKid $womenKid);

    public function ageOfWomenIs(Age $age);

}



final class Age

{

    private $value;



    public function __construct($value)

    {

        if (is_integer($value) === false) {

            throw new RuntimeException('Age value should be construct from an integer');

        }



        if ($value < 0) {

            throw new RuntimeException('Age value should be greater than or equal to 0');

        }



        $this->value = $value;

    }



    public function getValue()

    {

        return $this->value;

    }



    public function executeIfGreaterOrEqualTo(callable $callable, self $age)

    {

        if ($this->value >= $age->value) {

            $callable();

        }

        return $this;

    }

}



final class Gold

{

    private $value;



    public function __construct($value)

    {

        if (is_integer($value) === false) {

            throw new RuntimeException('Gold value should be construct from an integer');

        }



        if ($value < 0) {

            throw new RuntimeException('Gold value should be greater than or equal to 0');

        }



        $this->value = $value;

    }



    public function getValue()

    {

        return $this->value;

    }

}



class Conan implements Hero

{

    private $gold;

    private $giveGold = false;



    public function __construct(Gold $gold)

    {

        $this->gold = $gold->getValue();

    }



    public function askGoldByWomen(Women $women)

    {

        if ($this->gold < 2) {

            return;

        }



        $hero = clone $this;

        $women->ageIsAskedByHero($hero);



        if ($hero->giveGold === true) {

            $this->gold = $this->gold - 2;

            $women->giveGoldByHero(new Gold(2), $hero);

            echo "[Hero] I still have " . $this->gold . " gold\n";

        }

    }



    public function askGoldByKid(Kid $kid)

    {

        if ($this->gold < 1) {

            return;

        }



        $this->gold = $this->gold - 1;

        $kid->giveGoldByHero(new Gold(1), $this);

        echo "[Hero] I still have " . $this->gold . " gold\n";

    }



    public function askGoldByWomenKid(WomenKid $womenKid)

    {

        if ($this->gold === 0) {

            return;

        }



        $hero = clone $this;

        $womenKid->ageIsAskedByHero($hero);



        if ($hero->giveGold === true) {

            $gold = $this->gold;

            $this->gold = 0;

            $womenKid->giveGoldByHero(new Gold($gold), $hero);

            echo "[Hero] I still have " . $this->gold . " gold\n";

        }

    }



    public function ageOfWomenIs(Age $age)

    {

        $age->executeIfGreaterOrEqualTo(

            function () { $this->giveGold = true; },

            new Age(18)

        );

    }

}





class Gamin implements Kid

{

    public function giveGoldByHero(Gold $gold, Hero $hero)

    {

        echo "Goood i can buy candies for " . $gold->getValue() . " gold\n";

    }

}



class BBardot implements Women

{

    private $age = 80;



    public function giveGoldByHero(Gold $gold, Hero $hero)

    {

        echo "I can feed pet with food cost " . $gold->getValue() . " gold\n";

    }



    public function ageIsAskedByHero(Hero $hero)

    {

        $hero->ageOfWomenIs(new Age($this->age));

    }

}





class Gamine implements WomenKid

{

    private $age = 9;



    public function giveGoldByHero(Gold $gold, Hero $hero)

    {

        echo "I can trick for " . $gold->getValue() . " gold\n";

    }



    public function ageIsAskedByHero(Hero $hero)

    {

        $hero->ageOfWomenIs(new Age($this->age * 10));

    }

}



$conan = new Conan(new Gold(300));

$gamin = new Gamin;

$bbardot = new Bbardot;

$gamine = new Gamine;



$conan->askGoldByKid($gamin);

$conan->askGoldByWomen($bbardot);

$conan->askGoldByWomenKid($gamine);

[Xenos]

Oui mais une limitation dans PHP ne permet pas de le faire proprement, le plus propre et donc d'avoir un interface composé.

Pourquoi ce serait "sale" ça?

Code PHP :

<?php 

class Hero {

    public function getGold($asker) {

        $amount = $asker instanceof Women && $asker instanceof Kid ? $this->gold : $asker instanceof Women ? 2 : $asker instanceof Kid ? 1 : 0;

        $this->gold -= $amount;

        return $amount;

    }

} 

[srm]

L'interface composée que tu proposes ne respecte pas la spec. C'est donc bien qu'il existe une spec qui n'est pas codable avec ta méthode. 
Moi, dans ma spec, j'ai dit "si c'est une A et une B et une C, il doit se passer ça", et non "si c'est une ABC, il doit se passer ça" (note: tu implémentes aussi BCA, BAC, CAB, CBA, ACB?)

Non pour BCA, BAC etc.
La spéc exacte n'est pas possible en PHP mais dans les autres langages si, donc je la change un peu c'est tout.
Ta méthode on n'a pas de contrat passé avec la méthode, donc bug potentiel.

c'est du à une limitation de PHP, c'est pas de ma faute à moi

Donc tu attends du langage qu'il se plie à ta façon de coder?

Non je fais avec le moins pire de ce qui est possible avec le langage. C'est toi qui n'accepte pas que je ne respecte pas à la lettre la spec, moi je décide de la changer un peu car au final je m'en fou qu'elle soit respecté à la lettre ça ne m'apporte rien.

En East le postulat "on ne retourne jamais autre chose que this" et pour simplifié à quelqu'un qui débute en East.
Toutes les classes qui ont un comportement et qui donc réagisse à l'environnement (qui sont donc des classes qui implémentent forcément une interface) doivent retourner this sur leurs méthodes publiques. Un valueObject n'a aucun comportement ou ne sais pas réagir à son environnement, il ne fait que transporter une donnée.

Ca pour moi, c'est vaseux, et ça sent le "je return parce que ça m'arrange". En plus, Age a une méthode executeIfGreaterOrEqualTo() ce qui lui donne plus que la simple responsabilité de stocker la valeur: il ne fait pas que transporter la valeur.

C'est encore une fois du à une limitation de PHP, dans un autre langage tu auras surchargé l'opérateur >= et tu auras du coup tout de suite vu que tu avais bien une valeur.
PHP ne le permet pas, donc on fait avec.

D'ailleurs, vu que Age n'implémente pas d'interface, elle n'est pas testable. Et vu que tu l'utilises en typehinting, t'es pas en Interface Seggregation.

On ne test pas un valueObject, il n'a pas de comportement. C'est comme si tu voulais tester un int ou avoir une interface sur un int.

Tous les valueObject n'ont pas besoin d'être comparer, mais pour ceux pour qui c'est le cas tu peux parfaitement faire une abstract NumberComparison

Tu fais pas d'héritage multiple en PHP, alors comment t'appliques la même approche à executeIfGreaterOrEqualTo, executeIfGreaterThan, executeIfLessOrEqualTo, executeIfLesThan?

J'ai bien nommé NumberComparison pour une bonne raison, il y aurait toutes les méthodes dedans.

Tu clones le héro, donc son or est infini. J'avais demandé à Conan de donner son argent, pas l'argent d'un clone
→ Teste et tu verras

[edit] Je reste, puisqu'il y a eu correction entre temps...
Okay, c'est fonctionnel. Mais des clones dans tous les sens, sur de la prod, tu penses pas que le serveur va souffrir pour rien? Si on a besoin d'un dialogue exclusif, je dis pas, mais le reste du temps?!

Quand tu es dans une discussion "je te demande de l'or, tu me demande mon âge, je te donne de l'or" tu es obligé d'être dans un dialogue exclusif, sinon tu peux avoir plein de bug qui sortent de nul part. Au niveau perf ça coute pas grand chose.

A force de faire:
• L'appelant stocke l'état dans lequel il est
• L'appelant appelle l'appelé en se passant comme $caller
• L'appelé fait ses traitements
• L'appelé appelle l'appelant en lui passant son résultat
• L'appelant restaure l'état dans lequel il était et traite le résulta
t
T'as pas juste l'impression de recoder ce que return fait déjà? Dans les 0.1% de cas où c'est utile parce qu'on veut modifier une de ces étapes, je dis pas, c'est bien de faire comme ça parce qu'il n'y a pas de mot clef du langage pour le faire, mais dans les 99.9% du reste du temps, c'est juste lourd pour rien.

La question c'est pas de se demander si c'est utile ou pas, la question c'est de coder proprement.
Est-ce que tu te dis "là je ne mets pas d'interface ça ne sert à rien" ? Si oui bah c'est pas bon du tout.

[edit 2]: pour toi instanceof c'est choquant, mais is_integer ca ne l'est pas?

C'est un valueObject, sont but c'est de vérifier que la donnée qu'on lui donne correspond à ce qu'il attend, pas le choix du là encore à une limitation de PHP.
En PHP7 on pourra s'affranchir de ce test avec le typehint int.

[srm]

Pour PetitePeste j'étais en train d'y réfléchir justement, je savais que j'avais viré une étape et je regardais le code du phpTour car je pensais qu'il avait trouvé un autre moyen de faire, donc voici le mien :

<?php



interface GiveGoldByHero

{

    public function giveGoldByHero(Gold $gold, Hero $hero);

}



interface Kid extends GiveGoldByHero { }



interface Women extends GiveGoldByHero

{

    public function ageIsAskedByHero(Hero $hero);

}



interface WomenKid extends Women, Kid { }



interface Hero

{

    public function askGoldByWomen(Women $women);

    public function askGoldByKid(Kid $kid);

    public function askGoldByWomenKid(WomenKid $womenKid);

    public function ageOfWomenIs(Age $age);

}



final class Age

{

    private $value;



    public function __construct($value)

    {

        if (is_integer($value) === false) {

            throw new RuntimeException('Age value should be construct from an integer');

        }



        if ($value < 0) {

            throw new RuntimeException('Age value should be greater than or equal to 0');

        }



        $this->value = $value;

    }



    public function getValue()

    {

        return $this->value;

    }



    public function executeIfGreaterOrEqualTo(callable $callable, self $age)

    {

        if ($this->value >= $age->value) {

            $callable();

        }

        return $this;

    }

}



final class Gold

{

    private $value;



    public function __construct($value)

    {

        if (is_integer($value) === false) {

            throw new RuntimeException('Gold value should be construct from an integer');

        }



        if ($value < 0) {

            throw new RuntimeException('Gold value should be greater than or equal to 0');

        }



        $this->value = $value;

    }



    public function getValue()

    {

        return $this->value;

    }

}



class Conan implements Hero

{

    private $gold;

    private $giveGold = false;



    public function __construct(Gold $gold)

    {

        $this->gold = $gold->getValue();

    }



    public function askGoldByWomen(Women $women)

    {

        if ($this->gold < 2) {

            return;

        }



        $hero = clone $this;

        $hero->giveGold = false;

        $women->ageIsAskedByHero($hero);



        if ($hero->giveGold === true) {

            $this->gold = $this->gold - 2;

            $women->giveGoldByHero(new Gold(2), $hero);

            echo "[Hero] I still have " . $this->gold . " gold\n";

        }

    }



    public function askGoldByKid(Kid $kid)

    {

        if ($this->gold < 1) {

            return;

        }



        $this->gold = $this->gold - 1;

        $hero->giveGold = false;

        $kid->giveGoldByHero(new Gold(1), $this);

        echo "[Hero] I still have " . $this->gold . " gold\n";

    }



    public function askGoldByWomenKid(WomenKid $womenKid)

    {

        if ($this->gold === 0) {

            return;

        }



        $hero = clone $this;

        $hero->giveGold = false;

        $womenKid->ageIsAskedByHero($hero);



        if ($hero->giveGold === true) {

            $gold = $this->gold;

            $this->gold = 0;

            $womenKid->giveGoldByHero(new Gold($gold), $hero);

            echo "[Hero] I still have " . $this->gold . " gold\n";

        }

    }



    public function ageOfWomenIs(Age $age)

    {

        $age->executeIfGreaterOrEqualTo(

            function () { $this->giveGold = true; },

            new Age(18)

        );

    }

}



class PetitePeste implements Women {

    public function ageIsAskedByHero(Hero $hero) {

    }

    public function racketer(Hero $hero) {

        // Ca pourrait venir d'un autre bout de code:

        // Dans un tel cas, bon courage pour débugger 

        $hero->ageOfWomenIs(new Age(42));



        $hero->askGoldByWomen($this);

    }

    public function giveGoldByHero(Gold $gold, Hero $hero) {

        echo "Thanks for your " . $gold->getValue() . " goldcoins, sucker!\n";

    }

}



$conan = new Conan(new Gold(300));



$peste = new PetitePeste();

$peste->racketer($conan);

var_dump($conan);

[Xenos]

Je t'ai donné un code qui réponds exactement à la spec. On peut ergoter sur le cas $amount=0 (faut-il retourner 0 ou null?), mais pour le reste, cela répond bien à ma spec. Ce que tu n'arrives pas à assimiler, c'est que ma méthode accepte n'importe quoi en entrée (c'est son contrat), et qu'elle retournera une quantité différente suivant qui l'a demandé (enfin, suivant le paramètre d'entrée, car au fond, rien ne garantis que ce paramètre d'entrée est l'appelant, le $this). Son contrat (le message auquel elle répond) n'est pas son algorithme interne de réponse.


Forcément, si tu choisis de ne pas respecter la spec, cela change des choses, mais cela te force à avouer que ta méthode ne permet pas de tout coder. Donc, j'ai bien trouvé ton cas impossible en East, et je remettrais en cause la méthodologie. Sauf que tu veux pas l'avouer. C'est pas grave de se tromper, mais s'obstiner ainsi, ça me semble problématique (ou de mauvaise foi). Je sais que plus on a investi dans un truc, plus c'est difficile de le lâcher ou d'avouer que c'était un échec/une erreur, mais faut savoir le faire: ça aide dans la vie.


Je suis d'accord: on ne teste pas un bean. Sauf que t'as pas fait un bean, puisque tu lui as collé une méthode. Force est de constater que cette méthode n'est pas testable.


Heu, les clone au niveau perf, je demande à voir... Bon, c'est vu: 4ms environ pour les 3 échanges, alors que le mien mets 1ms. Bon, okay, ealy optimization is evil, mais ne me sors plus l'argument que "Au niveau perf, c'est pas grand chose".

Est-ce que tu te dis "là je ne mets pas d'interface ça ne sert à rien" ? Si oui bah c'est pas bon du tout

C'est pas ce que tu viens de faire pour un faux-valueobject? Là, t'es en train de recoder ce que le langage propose déjà (les returns) juste parce que tu veux pas reconnaitre que c'est utile.


Okay, tu t'affranchis du test is_integer en PHP7. Pourquoi considères-tu que le test $i > 0 est de nature différente de is_integer($i) ou de $i instanceof Machin?

Okay, t'as protégé ton code contre la PetitePeste. Maintenant, la méthode Hero::ageOfWomenIs(Age $age) n'a aucun intérêt à être appelée par une classe externe. Tu penses pas que ça pollue inutilement l'API publique de ton interface?
Par ailleurs, si un autre héro (disons, Attila, oui c'est un héro byzare) ne fait pas la distinction entre l'age de ceux qui demandent de l'argent, tu fais comment? Tu te traînes la méthode ageOfWomenIs inutilement (vive le dead code)? Tu crées une nouvelle interface, et tu bricoles Conan ? Tu vas péter des tas de trucs...

[srm]

Bon je laisse tombé tu as gagné
Visiblement tu ne veux pas comprendre, tant pis pas grave

[Xenos]

"tu ne veux pas comprendre"

Je te retourne le compliment. Poses-toi juste la question "si c'est East qui était l'idée de départ, n'y aurait-il pas une raison (l'évolution par ex) pour que West ait finit par le supplanter?".

[srm]

De la petite lecture pour ta nuit si tu décides un peu de voir plus loin : http://lmgtfy.com/?q=+instanceof+bad+practice

[srm]

Tout le monde a oublié l'idée de départ c'est pas plus compliqué que ça.
Et plus le temps passe plus on y revient petit à petit.