SOLID – O: Open/Closed – Le méchant blog

Le deuxième principe de SOLID nous apprend qu’il faut être ouvert à l’extension mais fermé à la modification.

Mais qu’est-ce que cela veut bien pouvoir dire ? Comment faire ? Est-ce que c’est possible ? Fermé aux modifications ? Cela veut dire que je ne vais plus modifier mon code ? Comme disait mon bon vieux binôme lors de mes études : Oui et non.

Oui… et non, mais pensez YAGNI et KISS!

En « open/closed », le code n’est modifié que lorsque votre client vient avec un nouveau type de demande. Qu’est-ce qu’un type de demande ? Prenons par exemple une méthode qui reçoit un « Xml » en paramètre. Votre client demande qu’il puisse donner un « Yaml » ou un « Csv ». Si vous commencez à écrire un « switch » sur le type de l’argument, qu’il faut alimenter à chaque nouvelle demande, alors vous violez ce principe. Si, par contre, vous n’avez rien à changer, c’est gagné !

Si votre client vient avec une nouvelle demande dont il en a le secret, du style : « Je veux que mon xml soit filtré, nettoyé et colorié en rouge », alors là, dans tous les cas, vous allez devoir modifier le code, faire des nouvelles méthodes, les appeler, etc…

L’essentiel est de retenir ceci : Le principe « open/closed » se limite aux faits que vous auriez déjà pu rencontrer lors de vos développements. Et surtout, surtout, surtout (surtout !), n’essayez pas d’anticiper la demande du client. Pourquoi ? Tout d’abord, parce que cela alourdit votre code et votre architecture pour « rien », vu que pour le moment, on ne l’utilise pas. Ce qui vous fera peut-être perdre du temps lors de la modification du code. Ensuite, parce qu’il faut maintenir les tests unitaires pour ces cas-là (qu’on n’utilise pas). Et enfin, parce que le client a une sorte de sixième sens, et vous demandera toujours quelque chose que vous n’aviez pas prévu.

KISS (Keep it stupid simple) et YAGNI (You aint gonna need it) sont les maitres-mots même pour le principe « open/closed ».

Ce qu’il ne faut pas perdre de vue, c’est que si le client demande une modification, c’est souvent le moment de refactoriser le code pour « préparer » les futures demandes du même type. La bonne blague ! Je vous dis de « préparer l’avenir » alors que 3 phrases plus haut, j’abordais YAGNI et KISS ? Laissez-moi une chance de m’expliquer ! La parole est à l’avocat de la couronne, c’est à vous, maitre Sneyters.

Comment ne pas procéder ?

Voici un bref résumé d’un mois de développement sur une application révolutionnaire : Le File Importer 2000. Le nom est un peu (beaucoup) has-been, mais que voulez-vous…

jour 0 : Votre client vous demande de prendre un csv sur un ftp distant pour l’importer dans votre application. Facile ! Votre classe se connectera sur un ftp, va récupérer le fichier distant afin de le mettre dans un dossier local. En code très simplifié, cela donne:

class FileImporter{

    private $saveTo;
    private $config;

    public function __construct($saveTo, array $config)
    {
        $this->saveTo = $saveTo;
        $this->config = $config;
    }

    public function import($file)
    {
        $connectionId = ftp_connect($this->config['server']);
        ftp_login($connectionId, $this->config['user'], $this->config['password']);
        $return = ftp_get($connectionId, $this->saveTo . DIRECTORY_SEPARATOR . $file, $file, FTP_BINARY);
        ftp_close($connectionId);
        return $return;
    }
}

class FileImporter{

private $saveTo;

private $config;

public function __construct($saveTo, array $config)

{

$this->saveTo = $saveTo;

$this->config = $config;

}

public function import($file)

{

$connectionId = ftp_connect($this->config['server']);

ftp_login($connectionId, $this->config['user'], $this->config['password']);

$return = ftp_get($connectionId, $this->saveTo . DIRECTORY_SEPARATOR . $file, $file, FTP_BINARY);

ftp_close($connectionId);

return $return;

}

jour 14 : Vous avez fini et votre client vous dit qu’il se peut que le fichier ne soit plus sur un ftp, mais sur un compte dropbox. En bon exécutant, vous allez rajouter la notion de « dropbox » à côté du ftp.

class FileImporter{

    private $saveTo;
    private $config;

    public function __construct($saveTo, array $config)
    {
        $this->saveTo = $saveTo;
        $this->config = $config;
    }

    public function import($file, $isDropBox = false)
    {
        if($isDropBox){
            return $this->downloadFromDropBox($file);
        }
        return $this->downloadFromFtp($file);
    }

    private function downloadFromFtp($file)
    {
        //...
    }

    private function downloadFromDropBox($file)
    {
        //...
    }
}

class FileImporter{

private $saveTo;

private $config;

public function __construct($saveTo, array $config)

{

$this->saveTo = $saveTo;

$this->config = $config;

}

public function import($file, $isDropBox = false)

{

if($isDropBox){

return $this->downloadFromDropBox($file);

}

return $this->downloadFromFtp($file);

}

private function downloadFromFtp($file)

{

//...

}

private function downloadFromDropBox($file)

{

//...

}

Pour éviter une trop forte régression, j’indique que mon deuxième paramètre est optionnel. Cela m’évite de changer les appelants.

Jour 30 : Vos 2 types de récupération sont prêts et votre client vous dit qu’il est aussi possible qu’il le mette lui-même dans le dossier « /upload » de votre application. Pas bien grave, votre booléen se transforme en « int » sur lequel vous allez faire un switch pour savoir si on utilise le ftp, le compte dropbox ou le dossier « /upload ». Là, par contre, vous allez devoir modifier les appelants, c’est-à-dire ceux qui utilisent l’ancien système de booléen.

class FileImporter{

    const IMPORT_FTP = 1;
    const IMPORT_DROPBOX = 2;
    const IMPORT_FILESYSTEM = 3;

    private $saveTo;
    private $config;

    public function __construct($saveTo, array $config)
    {
        $this->saveTo = $saveTo;
        $this->config = $config;
    }

    public function import($file, $type)
    {
        switch($type)
        {
            case self::IMPORT_FTP:
                return $this->downloadFromFtp($file);
                break;
            case self::IMPORT_DROPBOX:
                return $this->downloadFromDropBox($file);
                break;
            case self::IMPORT_FILESYSTEM:
                return $this->downloadFromFileSystem($file);
                break;
            default:
                throw new Exception('Wrong type');
                break;
        }
    }

    private function downloadFromFtp($file)
    {
        //...
    }

    private function downloadFromDropBox($file)
    {
        //...
    }
    private function downloadFromFileSystem($file)
    {
        //...
    }
}

class FileImporter{

const IMPORT_FTP = 1;

const IMPORT_DROPBOX = 2;

const IMPORT_FILESYSTEM = 3;

private $saveTo;

private $config;

public function __construct($saveTo, array $config)

{

$this->saveTo = $saveTo;

$this->config = $config;

}

public function import($file, $type)

{

switch($type)

{

case self::IMPORT_FTP:

return $this->downloadFromFtp($file);

break;

case self::IMPORT_DROPBOX:

return $this->downloadFromDropBox($file);

break;

case self::IMPORT_FILESYSTEM:

return $this->downloadFromFileSystem($file);

break;

default:

throw new Exception('Wrong type');

break;

}

private function downloadFromFtp($file)

{

//...

}

private function downloadFromDropBox($file)

{

//...

}

private function downloadFromFileSystem($file)

{

//...

}

Et voilà, après un mois tout va comme sur des roulettes ! Par contre, alors que la demande est toujours du même type, à savoir « Ajouter un nouveau type d’input », vous devez modifier votre méthode, et parfois le code qui l’appelle. En tant que développeur, vous savez que modifier du code, c’est s’exposer à des problèmes de différents types:

problèmes de régression,
problèmes de maintenance de tests unitaires,
problèmes de « merge » de branches,
etc.

Comment procéder ?

Reprenons l’énoncé en mode « open/closed ».

Jour 0: Création de la classe. Rien ne change par rapport à la version « not open/closed » étant donné qu’on est aussi en mode KISS, on ne va rien prévoir du tout. Le code reste donc exactement:

class FileImporter{

    private $saveTo;
    private $config;

    public function __construct($saveTo, array $config)
    {
        $this->saveTo = $saveTo;
        $this->config = $config;
    }

    public function import($file)
    {
        $connectionId = ftp_connect($this->config['server']);
        ftp_login($connectionId, $this->config['user'], $this->config['password']);
        $return = ftp_get($connectionId, $this->saveTo . DIRECTORY_SEPARATOR . $file, $file, FTP_BINARY);
        ftp_close($connectionId);
        return $return;
    }
}

class FileImporter{

private $saveTo;

private $config;

public function __construct($saveTo, array $config)

{

$this->saveTo = $saveTo;

$this->config = $config;

}

public function import($file)

{

$connectionId = ftp_connect($this->config['server']);

ftp_login($connectionId, $this->config['user'], $this->config['password']);

$return = ftp_get($connectionId, $this->saveTo . DIRECTORY_SEPARATOR . $file, $file, FTP_BINARY);

ftp_close($connectionId);

return $return;

}

Jour 14: Ajout de « DropBox ». C’est là qu’il faut se poser la bonne question : On nous demande que l’input puisse être, soit un ftp soit un compte « dropbox ». Faisons preuve d’un peu d’abstraction (mot-clé), et utilisons l’injection de dépendance (encore un mot clé). Ainsi, nous allons créer une interface « InputStream » qui sera implémentée par les classes « Ftp » et « DropBox ».

Notre méthode se voit doté d’un nouvel argument qui est un objet de type « InputStream ». Du coup, le code sera le même pour la classe « Ftp » et « DropBox » et ne sera plus modifié (fermé à la modification) tant que la demande ne change pas. De plus, elle fonctionnera avec d’autres types d’input qui implémente l’interface « InputStream » (elle est ouverte à l’extension).

//Implémenté par les classes DropBox et Ftp
interface InputStream
{
    public function getContent($file);
}

//Une classe pour la gestion de Ftp
class FtpStream implements InputStream{
    public function getContent($file){
        //...
    }
}

//Une classe pour la gestion de DropBox
class DropBoxStream implements InputStream{
    public function getContent($file){
        //...
    }
}

class FileImporter
{
    private $saveTo;

    //Même plus besoin de config!
    public function __construct($saveTo)
    {
        $this->saveTo = $saveTo;
    }

    public function import(InputStream $fileStream, $file)
    {
        file_put_contents($this->saveTo . DIRECTORY_SEPARATOR . $file, $fileStream->getContent($file));
    }
}

//Implémenté par les classes DropBox et Ftp

interface InputStream

{

public function getContent($file);

}

//Une classe pour la gestion de Ftp

class FtpStream implements InputStream{

public function getContent($file){

//...

}

//Une classe pour la gestion de DropBox

class DropBoxStream implements InputStream{

public function getContent($file){

//...

}

class FileImporter

{

private $saveTo;

//Même plus besoin de config!

public function __construct($saveTo)

{

$this->saveTo = $saveTo;

}

public function import(InputStream $fileStream, $file)

{

file_put_contents($this->saveTo . DIRECTORY_SEPARATOR . $file, $fileStream->getContent($file));

}

Bien sûr, cette modification de code, en engendre une autre: Les appelants. Souvenez-vous, on faisait la même chose en mode « not open/closed », mais 15 jours plus tard (le jour 30). Il y a donc des chances pour qu’il y ait moins de code impacté.

Jour 30: Ajout du dossier local : On crée juste une nouvelle classe « LocalFolder » qui implémente l’interface « InputStream » et qui représente un dossier local (« /upload » dans notre cas). Le code de ma méthode n’a pas été modifié, mes tests unitaires non plus. J’ai donc peu de chance d’avoir des problèmes de régression.

Et voilà, notre classe répond bien au principe « open/closed » ! Cependant, comme je vous l’ai dit en début d’article, c’est « closed » uniquement sur ce qu’on connaît, car il est impossible d’anticiper les demandes d’un client… Il sera toujours plus malin que vous.

Dernier truc pour la route

Un petit truc, qui ne marche pas à tous les coups, mais qui peut vous aider : Lorsque le client vous demande d’ajouter une fonctionnalité, mais que vous devez modifier du code, c’est que votre code n’est peut-être pas si « open/closed ». Pour notre File Importer 2000, le client nous demande d’ajouter différents types d’input possibles, il faut donc ajouter de nouvelles classes, il ne faut pas modifier la méthode qui gère l’import. « Add new feature » n’est pas « modify existing logic ».

Ça vaut ce que ça vaut, mais ça ne coûte rien de se poser la question…

Rejoindre la conversation

10 commentaires

Jérémy a dit :

19 décembre 2013 à 16 h 45 min

J’attends la suite avec impatience 😀

Répondre
__fabrice a dit :

19 décembre 2013 à 23 h 46 min

Bonjour,

Super site, et supers exemples 🙂

Je suis tout à fait ok pour ces pratique de dev, c’est la base;
Donc, tu devras faire ? :

$a = new FileImporter(‘/tmp’);
$a->import(new LocalFolder, « toto.txt »);
?

Fabrice

Répondre
Nicolas a dit :

20 décembre 2013 à 11 h 35 min

@Jérémy: Ce sera pour lundi ou mardi… Stay tuned 😉
@__fabrice: Oui, en précisant le dossier upload dans le constructeur de LocalFolder, comme ceci:

$a->import(new LocalFolder(‘/upload’), “toto.txt”);

Répondre
Mika a dit :

21 décembre 2013 à 14 h 33 min

Intéressant et pratique. J’ai hâte de lire la suite

Répondre
__fabrice a dit :

21 décembre 2013 à 21 h 26 min

Merci Nicolas,

Par contre, je ne comprends pas :
$a->import(new LocalFolder(‘/upload’), “toto.txt”);

C’est pourtant la classe FileImporter qui gère le dossier $saveTo et non l’interface.

Merci
Fabrice

Répondre
Nicolas a dit :

23 décembre 2013 à 1 h 21 min

@__fabrice: L’attribut du constructeur permet de savoir dans quel dossier on va sauver le fichier ($saveTo = /tmp)

Le premier argument de import indique où on va chercher ce fichier (/upload). Le deuxième indique quel est le fichier (toto.txt) à télécharger.

Est-ce que je répond à ta question? 🙂

Répondre
Joris a dit :

27 décembre 2013 à 3 h 19 min

Si j’utilise 1 seul dossier pour tous mes imports c’est ok.
Mais si je gère mes imports avec 2 dossiers ou plus :
/tmp/ftp_files et /tmp/dropbox_files, etc…
Dans ce contexte
à mon avis c’est une erreur d’indiquer le dossier de réception dans le constructeur.
Car pour une instance de FileImporter, le dossier de réception est gravé dans le marbre.

Si je veux importer fichier1 dans le dossier /tmp/ftp_files puis le fichier2 dans /tmp/dropbox_files
je suis obligé d’instancier 2 fois FileImporter car 2 dossier différents.
Il est alors préférable d’indiquer le dossier de réception dans la méthode importer($file, $saveTo).
…ou sinon créer un setter pour le dossier local mais ca alourdit l’usage.
Qu’en pensez-vous ?

Répondre
Nicolas a dit :

27 décembre 2013 à 16 h 40 min

Bonjour Joris,

Tout à fait d’accord avec vous! L’exemple donné est très simplifié et ne fonctionne qu’avec un dossier. Disons que c’est la demande du client 🙂

Si vous devez gérer plusieurs dossiers, votre manière de faire est tout à fait correcte, c’est-à-dire 2 arguments dans la méthode importer et plus de variable « $this->saveTo ».

Répondre
Joris a dit :

2 janvier 2014 à 18 h 47 min

en fait je voulais vérifier que mon raisonnement est valable dans le cas où faudrait gérer plusieurs dossiers ^^

Répondre
Ping : Solid – D : Dependency inversion | Le méchant blog

SOLID – O: Open/Closed

Oui… et non, mais pensez YAGNI et KISS!

Comment ne pas procéder ?

Comment procéder ?

Dernier truc pour la route

Rejoindre la conversation

Annuler la réponse

Laisser un commentaire