Compass : East Oriented

[srm]

En parallèle, au sens de PHP, est à entendre comme A l'insu de la classe appelante (mais peut parfaitement être un vrai parallèle dans d'autres langages).
L'idée était de souligner que la fonction appelée peut, de toute façon, retourner "merde" à son appelant (sous la forme d'un null).

Oui enfin si ton getter fait un : $this->goAway()
Je pense que pas mal de dev vont te dire WTF un getter retourne une variable et n'a aucune autre logique.
Et ils auront raison, alors que justement en East vu que tu demandes juste quelque chose, tu n'imposes rien au niveau réaction de l'objet appelé.

D'ailleurs, question subsidiaire: comment réaliser des tests unitaires quand les méthodes sont des boites noires complètes qui ne renvoient qu'une référence d'elles-mêmes? Quoiqu'on peut répondre la notion de test n'a pas lieu d'être puisqu'on 'je sais ce que je veux, et je *fais confiance pour que ce soit fait*'... Mais si jamais le code part en cacahouète, je serai curieux de savoir comment on débug cela...

Les tests sont beaucoup plus simple à faire c'est mageekguy lui même qui le dit, qui est le créateur de atoum je rappel.
Tous les objets ont des interfaces donc c'est très facile à mocker et au final le test consiste à vérifier que les messages sont bien passés. 

[srm]

Si j'ai bien compris ton besoin, c'est genre ça que tu veux :

<?php



class FileAggregator implements ArrayIterator

{

    private $files = [];



    public function add(File $file, $lineNumber, FileUploader $fileUploader)

    {

        switch ($file->type) {

            case 'pdf':

                // Only one pdf allowed

                // We erase all other files

                if (isset($this->files[$lineNumber]['pdf']) === true) {

                    $this->files[$lineNumber] = ['pdf' => $this->files[$lineNumber]['pdf']];

                    $fileUploader->fileRejected($file, 'PDF already exist on this line');

                } else {

                    $this->files[$lineNumber][$file->type] = $file;

                }

                

                break;



            case 'dxf':

            case 'dwg':

                if (isset($this->files[$lineNumber][$file->type]) === false) {

                    $this->files[$lineNumber][$file->type] = $file;

                }

                break;

        }

    }

    

    

    public function listValidFiles(FileUploader $fileUploader)

    {

        foreach ($this->files as $files) {

            if (isset($files['pdf']) === true) {

                foreach ($files as $type as $file) {

                    $fileUploader->upload($file);

                }

            }

        }

    }

}

 

class File

{

    public $filename;

    public $type;

}

 

class FileUploader

{

    public function thereIsAFileToUpload($lineNumber, File $file, FileAggregator $fileAggregator)

    {

        $fileAggregator->add($file, $lineNumber, $this);

    }

    

    public function fileRejected(File $file, $message)

    {

        echo 'File rejected ' . $message;

    }

    

    public function uploadAllFiles()

    {

        $fileAggregator->listValidFiles($this);

    }

    

    public function upload(File $file)

    {

        // Upload stuff for $file

    }

}

[Xenos]

Je vais reprendre

J'ai des tuyauteries, qu'on appelle des lignes ("Line"). Chacune possède un code unique ("LineBq").
On dispose d'un dossier que l'on veut uploader dans le logiciel. Dans ce dossier se trouvent des fichiers:
• Des documents PDF
• Des documents DXF, DWG
• D'autres documents inutiles

Les PDF doivent respecter une structure de nom: "LineBq_yyyy-mm-dd_Rev_Nom.pdf" (c'était dans le 1er post), où LineBq indique le numéro de la ligne à laquelle le PDF doit être rattaché, yyy-mm-dd est la date de création de ce PDF (note: le PDF est le plan de la tuyauterie), Rev est un "numéro" alphabétique de révision (A, B, C...Z, AA,...) et Nom, un nom quelconque.
Les DXF/DWG respectent un schéma similaire: LineBq_Nom.dxf/LineBq_Nom.dwg (pas de rev ni de date).

On souhaite n'uploader que les PDF et les DXF/DWG. Les autres documents (inutiles), sont ignorés (et on les liste dans un message d'erreur).
Pour une ligne, on ne peut uploader un DXF/DWG que si un (et un seul) PDF est uploadé en même temps, pour cette ligne.
Pour une ligne, on ne peut uploader qu'un et un seul PDF.

Du coup, j'étais partis sur quelque chose comme:

Class FolderUploader {

	FileUploader[] fups = {PDFUploader, DXFUploader, DWGUploader, OtherUploader};

	{File, LineBq}[] uploadables; // File & lineBq

	{File, String}[] faileds; // File & raison de l'échec



	upload(Folder fold)

		foreach (File f in fold) {

			foreach (FileUploader fu in fups) {

				fu->upload(f, uploadables, faileds);

			}

		}

	}

}

Class PDFUploader {

	upload(File f, {File, LineBq}[] oks, {File, String}[] fails) {

		

	}

}

Class DXFUploader {

	upload(File f, {File, LineBq}[] oks, {File, String}[] fails) {

		

	}

}

Class DWGUploader {

	// Similaire à DXFUploader

}

Class OtherUploader {

	upload(File f, {File, LineBq}[] oks, {File, String}[] fails) {

		

	}

}

[srm]

Donc mon exemple est bon en gros

[Xenos]

Désolé, j'avais raté cette page.

Sauf que dans le else $this->files[$lineNumber][$file->type] = $file;, il faut virer les DXF/DWG qui ont pu être uploadés, ansi que l'éventuel PDF déjà uploadé.
Ton $file->type, c'est pas un getter sous la forme d'un attribut publique?

Puis après, l'idée serait de réutiliser le même code pour upload un seul fichier ('sont exigeants les clients, hein?! )... Et là, sans contrainte de nom (on sait pour quelle ligne le fichier PDF/DWF/DWG doit être uploadé)...

Bref, je reste sceptique au vu de la floraison de "case". Le code me semble pas si réutilisable qu'il ne l'est dit.

[srm]

Pour le else tu as dit que l'on avait le droit d'uploader dans n'importe quel sens.
Pour $file->type c'est un valueobject.

Pour le reste, j'ai l'impression que tu veux pas être convaincu. Puisque tu commences par dire :
"Perso, en essayant East là dessus, j'ai vite terminé en un immonde sac de noeuds..."
Je te montre que tu as mal codé et que ça vient pas de East.

Puis tu me trouves autre chose à dire

[Xenos]

D'accord, ok, j'avais pas compris que le listValid se charge de faire le tri "y'avait un PDF/y'en avait pas". C'est vrai qu'il faut s'habituer au fait que ce soit pas lisible dans sa globalité (mais, c'est voulu, donc faut juste se caler l'esprit dans ce sens là, d'accord).

J'objecte, oui, parce qu'avant de me lancer dans une technique assez lourde, j'aimerai la questionner un max.
D'ailleurs, la classe File ne me semble pas East-Oriented?! Car File est forcée de retourner une chaîne String comme type, voire une classe castable en String (sinon, le switch est cassé).
Du coup, faudrait une méthode, dans file, passFileTypeTo(FileAggregator fg)...

Voire même, une classe dédiée...


Du coup, j'arriverai à:

<?php



interface IFileTypeCommand {

    public void run(String $type);

}

    

class FileAggregator_FileTypeCommand implements IFileTypeCommand() {

    private FileAggregator $fg;

    private FileUploader $fileUploader;

    private File $file;

    private $lineNumber;

    

    public __construct(FileAggregator $fg, File $file, $lineNumber, FileUploader $fileUploader) {

        $this->fg = $fg;

        $this->fileUploader = $fileUploader;

        $this->file = $file;

        $this->lineNumber = $lineNumber;

    }

    public void run(String $type) {

        $fg->addTyped($this->file, $type, $this->lineNumber, FileUploader $this->fileUploader) {

    }

}



class FileAggregator implements ArrayIterator

{

    private File[] $files = [];

 

    public void add(File $file, $lineNumber, FileUploader $fileUploader) {

        $file->pushTypeTo(

            new FileAggregator_FileTypeCommand($this, $file, $lineNumber, $fileUploader));

    }

    

    public void addTyped(File $file, $type, $lineNumber, FileUploader $fileUploader) {

        switch ($type) {

            case 'pdf':

                if (isset($this->files[$lineNumber]['pdf']) === true) {

                    $previous = $this->files[$lineNumber]['pdf'];

                    $previous['fileUploader']->fileRejected(

                        $previous['file'], 'Two PDFs for the same line');

                    $fileUploader->fileRejected($file, 'PDF already exist on this line');

                    unset($this->files[$lineNumber]);

                } else

                    $this->files[$lineNumber]['pdf']

                        = ['fileUploader' => $fileUploader, 'file' => $file];

 

                break;

 

            case 'dxf':

            case 'dwg':

                if (isset($this->files[$lineNumber][$file->type]) === false)

                    $this->files[$lineNumber][$file->type] = array();

                $this->files[$lineNumber][$file->type][]

                    = ['fileUploader' => $fileUploader, 'file' => $file];

                break;

        }

    }

 

    public function listValidFiles(FileUploader $fileUploader)

    {

        foreach ($this->files as $files) {

            if (isset($files['pdf']) === true) {

                foreach ($files as $type as $file) {

                    $fileUploader->upload($file);

                }

            }

            else if (count($files) > 0) { // equiv: isset ['dxf'] || isset ['dwg']

                foreach ($files as $type as $file) {

                    $fileUploader->fileRejected($file, 'No PDF for this line');

                }

            }

        }

    }

}

 

class File

{

    public $filename;

    private $type;

    

    public pushTypeTo(FileTypeCommand $fc) {

        $fc->run($this, $this->filename);

    }

}

 

class FileUploader

{

    public function thereIsAFileToUpload($lineNumber, File $file, FileAggregator $fileAggregator)

    {

        $fileAggregator->add($file, $lineNumber, $this);

    }

 

    public function fileRejected(File $file, $message)

    {

        echo 'File rejected ' . $message;

    }

 

    public function uploadAllFiles()

    {

        $fileAggregator->listValidFiles($this);

    }

 

    public function upload(File $file)

    {

        // Upload stuff for $file

    }

}

Mais du coup, FileAggregator_FileTypeCommand juste dédiée à récupérer le $type... Je trouve cela encombrant.
Ou alors, la règle East du "no return / no getter" est à pondérer et non à appliquer strictement partout ?

Finalement, ce qui me gène (outre la verbosité), c'est surtout que je ne vois pas ce que cela apporte, puisque get* ou "push*" (ou similaire), la classe appelée se retrouve quand même contrainte: soit par le typage de retour du getter, soit par le typage des paramètres de la classe où "pusher". La seule liberté ajoutée serait donc de ne pas pusher, mais du coup, cela me semble se rapprocher d'un "je sais ce que je veux, mais je prends le risque que ce soit juste pas fait", ce qui (au vu de mon expérience dans des associations IRL ) finit souvent en "rien ne se passe".

[srm]

File est un value object.
Donc tu vas t'assurer à la création des propriétés qu'elles sont au format choisis.

No return / no getter est à appliqué partout mais...
Pas partout dans le sens partout
Partout dans les méthodes qui ont des interactions avec d'autres objets.

Donc partout sauf dans ces cas là :
valueobject
méthode privé
potentiellement aussi sur les méthodes protégée, on est encore en train d'en discuter

Bah là par exemple si je décide que les règles métiers changent pour déterminer quel cas est valide pour uploader des fichiers. Si par exemple j'ai le droit à deux DXF pour 1 PDF il me suffit de modifier mon FileAggregator et rien d'autre

[Xenos]

Ah, okay, effectivement, avec ces deux règles, cela change des choses
De ce que j'avais lu des liens que tu as présentés (ou alors, c'était suite à une recherche web? je ne sais plus...), la règle était "pas de data, que des messages", donc j'en avais déduis "pas de value object".

Ca répond à l'autre question que j'allais d'ailleurs poser: pourquoi peut-on faire un [] (getter) sur un array ? Si les array[] sont considérés comme des value objects, c'est réglé. Après, en C++, l'opérateur [] se surcharge (c'est un opérateur comme un autre), donc faire de l'array un value object est plus délicat comme point de vue...

Bon, après, je ne suis quand même pas encore top-convaincu, surtout que j'aurai tendance à sortir les code des case dans des classes séparées (pour avoir, par exemple, un FileAggregator->switchCases[], liste chaînée des classes issues du case). Cela impliquerait de leur passer File $file, $type, $lineNumber, FileUploader $fileUploader, FileAggregator $this en paramètre... Cela commence à en faire, des paramètres (surtout si ces classes ne sont utilisables que par le FileAggregator: j'ai d'autres endroits dans le projet où on peut uploader fichier par fichier, et cette fois sans nomenclature de nom, et avec d'autres types de restrictions).

J'en reviens à ma dernière question: existe-t-il un projet industriel (maintenable) codé en East?


Quoique si, j'en ai une autre de question:
Tous les appels à des function(parameters...) { ... return X; } sont remplacés par des appels à function(resultTreatment, parameters...) { ... resultTreatment->(parameters..., X); } où la classe resultTreatment récupère la suite des opérations à effectuer (en lisant le code, tous les changements West → East se résument à ce genre de remplacements), c'est bien cela?
Est-ce vraiment plus souple? Car outre l'apparition d'une nouvelle classe (ou interface, implémentée par l'appelant), on bascule en fait le typage du retour de function(parameters...) vers un typage de resultTreatment->(parameters..., X) ?!

Dans l'exemple, FileAggregator est dépendant de FileUploader->fileRejected() ? L'ajout de dépendances n'empiète pas sur l'évolutivité du code?
Maintenant, okay, je crois que je visualise mieux comment cela marche, mais le pourquoi m'échappe encore...

[srm]

Je ne vois pas l'intérêt de sortir du switch case. Ça apporterait quoi ?
Tu aurais un autre FileAggregator alors si ils font pas le même boulot.
Peux être même aucune aggrégation.

Personne connait East, donc non pas de projet industriel.

C'est en gros ça, mais c'est pas mal schématiser quand même.
Bah oui c'est plus souple, souviens toi du Cas 3

FileAggregator n'est pas dépendant de FileUploader, il sait travailler avec c'est différent.
FileAggregator n'a pas besoin de FileUploader pour travailler.