Hello !

Certaines solutions plutôt simples à formuler sur le papier (et donc d'un point de vue Orienté Objet) se retrouvent incroyablement complexifiées une fois que l'on a fait le choix de les traduire dans un langage orienté objet. C'est à croire que ces fameux langages de programmation dont on nous vante les mérites depuis plusieurs lustres ne nous aident pas vraiment à écrire du code orienté objet. Ou alors c'est la faute à ces croyances aujourd'hui devenues tendancielles qui ne font qu'encourager l'usage de méthodes alambiquées et complètement contre-productives : je pense notamment à dynamic_cast et ses don'ts. Mais peut-être tout simplement est-ce moi qui m'égare de la solution à la problématique. Voyons voir ensemble de quoi il s'agit.

[1/2] Problématique

Je dispose des deux classes ci-après, toutes deux extraites d'un code plus étoffé.

class UndoCommand
{
public:
    explicit UndoCommand() = default;
    virtual ~UndoCommand() = default;

    /**
     * Returns a description of this command.
     * The default implementation merely returns "UndoCommand".
     */
    virtual std::string description() const;

    virtual void undo() = 0;
    virtual void redo() = 0;
};

class UndoSequence : public UndoCommand
{
private:
    std::vector<std::unique_ptr<UndoCommand>> m_subCommands;

public:
    UndoSequence();
    ~UndoSequence();

    void add(UndoCommand *command);

    /**
     * Returns this command description.
     * The returned value is "UndoSequence[N]", where N is the number of sub-commands.
     */
    std::string description() const override;

    /**
     * Undoes all sub-commands in reverse registration order,
     * i.e. commands are executed in the following order:
     *     subCmd[N-1] -> subCmd[N-2] -> ... -> subCmd[1] -> subCmd[0], N being the number of sub-commands.
     */
    void undo() override;
    /**
     * Redoes all sub-commands in registration order,
     * i.e. commands are executed in the following order:
     *     subCmd[0] -> subCmd[1] -> ... -> subCmd[N-2] -> subCmd[N-1], N being the number of sub-commands.
     */
    void redo() override;
};

Jusque là rien de bien compliqué. Maintenant ce que j'aimerais faire c'est créer une classe Stringifier qui, étant donnée une commande, retournerait une représentation textuelle de celle-ci : libre ensuite à chaque implémentation de "stringifier" la commande à sa guise. On en arrive donc à :

class UndoStringifier {
    NCPP_DISABLE_COPY(UndoStringifier)

public:
    UndoStringifier() = default;
    virtual ~UndoStringifier() = default;

    virtual std::string stringify(const UndoCommand &cmd) = 0;
};

Et c'est là que les problèmes commencent

• en particulier étant donné que la classe UndoSequence possède des attributs et fournit des services dont sa mère (classe parente) n'est pas au courant.

En théorie il n'y a pas vraiment de problèmes puisque l'on pourrait utiliser dynamic_cast dans les implémentations concrètes de UndoStringifier. Mais il ne faut surtout pas car

Croyance tendancielle

dynamic_cast c'est le mal.

Pour autant toutes les autres implémentations auxquelles j'ai pensées sont au mieux impertinentes, au pire elles complexifient le code et rajoutent des liens de dépendence inutiles.

[2/2] Autres Solutions

Solution 1

Fusionner les deux classes. Pas très intéressante comme solution car lesdites classes ne fournissent pas le même service. Les seules relations envisageables entre celles-ci sont l'héritage et l'agrégation (en l'occurrence ici la composition).

Solution 2

Créer deux classes filles à partir de Stringifier :

• la première ayant un constructeur recevant en paramètre UndoCommand&
• la seconde  ayant un constructeur recevant en paramètre UndoSequence&
• chacune des deux "overridant" la fonction stringify délarée plus tôt

class UndoStringifier {
    NCPP_DISABLE_COPY(UndoStringifier)

public:
    UndoStringifier() = default;
    virtual ~UndoStringifier() = default;

    virtual std::string stringify(const UndoCommand &cmd) = 0;
    virtual std::string stringify(const UndoSequence &cmd) = 0; // voici la nouveauté
};

std::string UndoCommand::getString(const UndoStringifier &stringifier)
{
    return stringifier.stringify(*this);
}

Salut,

Déjà, une séquence n'est pas une commande : une séquence contient des commandes, qui sont mises dans un ordre bien particulier, de manière à pouvoir accéder à la dernière commande en premier lieu selon le système LIFO (une pile, en gros).

En très gros, tu aurais plutôt quelque chose comme

/*la classe de base pour les commandes */
class Command{ // oui, parce que, UndoCommand, ca te limite
               // au fait qu'elle soit "undoable"...
public:
    virtual void undo() = 0;
    virtual void redo() = 0;
};
/* la partie qui permet de défaire / refaire l'historique */
class CommandSequence{
public:
    void done(std::unique_ptr<Command> && d){
        done_.push(std::move(d));
    }
    void undo(){
        if(!done.empty()){
             auto & top = done_.top();
             top.get()->undo();
             undone_.push(std::move(top));
             done_.pop();
        }
    }
    void redo(){
        if(!undone_.empty()){
             auto & top = undone_.top();
             top.get()->redo();
             done_.push(std::move(top));
             undone_.pop();
        }
    }
    void undoAll(){
        while(!done_.empty())
            undo();
    }
private:
    std::stack<std::unique_ptr<Command>> done_;
    std::stack<std::unique_ptr<Command>> undone_;
}

Pour autant toutes les autres implémentations auxquelles j'ai pensées sont au mieux impertinentes, au pire elles complexifient le code et rajoutent des liens de dépendence inutiles.

Oui, dynamic_cast, c'est le mal, non tu n'en as pas besoin, et non les alternatives ne rajoutent pas forcément de dépendances inutiles.

Ici, tu n'en as pas besoin parce que toute commande (héritant de Command) devra tout simplement savoir comment se défaire (ou comment se rejouer).  A toi de faire en sorte que cela fonctionne.

Par exemple, dans un traitement de texte, tu pourrais avoir une commande "mise en gras" qui serait proche de

class BoldCommand : {
public:
    BoldCommand(Text & text,size_t  begin, size_t end):
    text_{text}, begin_{begin}, end_{end}{}
    void undo() override{
        /* 1: sélectionner le texte entre begin et end 
         * 2: supprimer le marquage "gras" sur la sélection
         */
    }
    void redo() override{
        /* 1: sélectionner le texte entre begin et end
         * 2: ajouter le marquage "gras" sur la sélectio
         */
    }
private:
    Text & text_;
    size_t begin_;
    size_t end_;
};

Ou tu pourrais avoir une commande qui s'occupe de l'insertion de texte, sous une forme proche de

class TextInsertionCommand : public Command{
public:
    TextInsertCommand(Text & text, Text const & added, size_t begin):
        text_{text}, added_{added}, begin_{begin}{}
    void undo() override{
         /* 1: aller à la position begin dans text_
          * 2: s'assurer que les added_.size() caractères
          * dans text_ correspondent à added
          * 3: supprimer le texte correspondant
          */
    }
    void redo() override{
         /* 1: aller à la position begin dans text_
          * 2: insérer text_ à la position trouvée
          */
    }
};

Si les fonctions virtuelles existent en C++, si le polymorphisme d'inclusion existe en conception, c'est justement pour des cas comme celui-ci:  Comme tu peux le remarquer, on a identifié deux comportements, à savoir undo et redo, puis on a identifié deux commandes clairement distinctes, à savoir la mise en gras d'une partie du texte et l'insertion du texte.

Et l'on s'est donc arrangé pour que les comportements undo et redo réagissent de la manière appropriée pour la commande concernée.

De cette manière, on se fout pas mal, quand on a affaire à un élément qui n'est connu que comme une "commande" (au sens tout à fait général), on sait (parce que cela aura été codé comme cela) que quelle que soit la commande "réelle" qui se cache derrière cette commande "générale", les comportements undo et redo réagiront de manière adéquate

Maintenant, il arrive très régulièrement que tu te trouves dans une situation dans laquelle tu as été "assez bête" que pour perdre le type réel de ta commande et que tu ne la connaisse plus que comme "une commande, tout ce qu'il y a de plus générique", alors que tu aimerais tellement pouvoir faire appel à une fonction spécifique d'une des commandes concrètes, si seulement tu avais la certitude que la commande que tu manipules était bel et bien du bon type.

Pas de panique!  Car, si tu as oublié le type réel (selon mon exemple: BoldCommand ou InsertTextCommand) de la commande, elle, elle s'en souvient parfaitement : une commande de type BoldCommand sait qu'elle est de type BoldCommand et non de type InsertTextCommand, et inversement, bien sur.

Et c'est là que la magie opère. Car, si tu fais en sorte que ce soit la commande qui appelle une fonction en se transmettant elle-même à cette fonction, tu as la certitude que la fonction en question recevra... le type réel de la commande.

Mieux encore, grâce à la surcharge des fonctions, tu peux tout à fait faire appel à "la même fonction" (comprend à une fonction portant le même nom) depuis BoldCommand et depuis InsertTextCommand, tout en ayant la certitude que ce sera la "bonne version" de cette fonction qui sera appelée.

Allez, un exemple pour te faire comprendre: rajoutons un comportement doIt à notre classe de base, et créons deux fonctions libres appelée doSomething.  L'une prendra une BoldCommand comme paramètre et l'autre prendra une InsertTextCommand comme paramètre.

En gros, cela ressemblerait à quelque chose comme

class Command{ // oui, parce que, UndoCommand, ca te limite
               // au fait qu'elle soit "undoable"...
public:
    virtual void undo() = 0;
    virtual void redo() = 0;
	virtual void doIt() = 0;
};
void doSomething(BoldCommand const & c){
    std::cout<<"doSomething called with a BoldCommand\n";
}
class BoldCommand : {
public:
    BoldCommand(Text & text,size_t  begin, size_t end):
    text_{text}, begin_{begin}, end_{end}{}
    void undo() override{
        /* 1: sélectionner le texte entre begin et end 
         * 2: supprimer le marquage "gras" sur la sélection
         */
    }
    void redo() override{
        /* 1: sélectionner le texte entre begin et end
         * 2: ajouter le marquage "gras" sur la sélectio
         */
    }
	void doIt() override{
	     // affiche "doSomething called with a BoldCommand"
	    doSomething(*this);
	}
private:
    Text & text_;
    size_t begin_;
    size_t end_;
};
void doSomething(TextInsertionCommand const & c){
    std::cout<<"doSomething called with a TextInsertionCommand\n";
}
class TextInsertionCommand : public Command{
public:
    TextInsertCommand(Text & text, Text const & added, size_t begin):
        text_{text}, added_{added}, begin_{begin}{}
    void undo() override{
         /* 1: aller à la position begin dans text_
          * 2: s'assurer que les added_.size() caractères
          * dans text_ correspondent à added
          * 3: supprimer le texte correspondant
          */
    }
    void redo() override{
         /* 1: aller à la position begin dans text_
          * 2: insérer text_ à la position trouvée
          */
    }
	void doIt() override{
	     // affiche "doSomething called with a TextInsertionCommand"
	    doSomething(*this);
	}
};

Avoues que c'est magique, non?

Ca, mon ami, c'est ce que l'on appelle le double dispatch.  L'une des implémentations "classiques" de ce phénomène est connue comme le patron de conception visiteur.

Fusionner les deux classes. Pas très intéressante comme solution car lesdites classes ne fournissent pas le même service. Les seules relations envisageables entre celles-ci sont l'héritage et l'agrégation (en l'occurrence ici la composition).

Et encore : les règles pour pouvoir envisager l'héritage sont tellement strictes (car il faut respecter le LSP) que l'agrégation devrait être choisie par défaut... A moins, bien sur, que l'on n'ait une bonne raison de faire autrement

Ceci étant dit, n'oublies pas le SRP et l'ISP dans l'histoire : fusionner deux classes en une seule n'a du sens que si tu gardes effectivement les deux classes d'origine.

Ceci dit, C++ ne t'interdit pas l'héritage multiple, et, pour être honnête, les langages comme C# ou java non plus. La seule différence, c'est qu'ils mentent au développeur lorsqu'il prétendent que le mot clé implements fait autre chose que de l'héritage publique

Et si tu peux, effectivement, créer deux classes clairement distinctes, fournissant des services totalement différents (au minimum par leurs noms), il n'y a absolument aucun problème à faire en sorte qu'une classe dérivée hérite de deux classes de base (à condition toutefois de respecter aussi bien le SRP que l'ISP )

Cela n'est toutefois pas suffisant puisque la résolution de surcharge (des fonctions) ayant lieu à la compilation, passer une référence sur UndoCommand en paramètre à stringify déclencherait dans tous les cas l'appel de la première version de la fonction, jamais la seconde. Pour appeller la seconde fonction, il faudrait passer une référence sur UndoSequence. On en revient donc au même problème que dans le précédent cas.

Les fonctions virutelles sont résolues à l'exécution uniquement (au travers de la vtable, pour être précis

Quand tu crées une fonction virtuelle, le compilateur crée une sorte de tableau dans lequel il fait crée une relation entre un nom de fonction et l'adresse mémoire à laquelle il faut aller pour y faire appel.

Quand tu redéfinis cette fonction dans une classe dérivée, le compilateur met à jour l'adresse mémoire à laquelle il faut aller pour la classe dérivée en question (et pour toutes les classes qui en hériteraient sans redéfinir la fonction, par la même occasion).

Et quand tu appelles cette fonction virtuelle depuis ton code, c'est toujours l'adresse mémoire de la classe "la plus dérivée" pour laquelle la fonction a été redéfinie qui est utilisée.

La seule situation dans laquelle cela peut ne pas marcher correctement, c'est si tu appelles une fonction virtuelle depuis le destructeur de la classe: Comme les destructeurs sont appelés dans l'ordre inverse de la création, c'est d'abord celui de la classe "la plus dérivée" qui est appelé, puis celui de sa (ses) classe(s) mère(s) directe(s).

Si bien que, si tu fais appel à une fonction virtuelle dans le destructeur, tu ne connais déjà plus le type réel de la classe dérivée d'origine

C'est expliqué en quelques mots, en prenant très certainement des raccourcis quelques peu douteux. Mais ca a le mérite d'être compréhensible

-
Edité par koala01 20 septembre 2017 à 0:02:15

J'apprécie ta réponse, d'où mon +1. Mais elle ne me satisfait pas.

Concernant le nommage, tu as en partie raison. En effet plutôt que d'utiliser la nomenclature :

• UndoCommand
• UndoSequence
• UndoStack

j'aurais pu utiliser :

• Command
• CommandSequence
• CommandStack

Mais toutes les commandes ne sont pas forcément "undoable". Et non je ne fais pas allusion au pattern State : je parle bien d'une commande qui ne possède que la fonction do(). Les commandes étant donc "doable" par défaut, le fait de préfixer les classes par Undo permet de mettre l'accent sur le fait que la commande est "undoable". Mais semble-t-il, cela n'a pas eu l'effet escompté.

Aussi, CommandSequence est bien une commande. Peut être est-ce le terme utilisé qui est trompeur mais par UndoSequence (ou CommandSequence), je designe tout simplement une macro commande : une commande qui est composée d'autres commandes. Ce que tu appelles donc CommandSequence dans ton post est ce que j'appelle UndoStack (ou CommandStack si tu veux). Je n'en ai pas parlé dans le post car cela n'était pas nécessaire.

Par ailleurs la solution que tu proposes (celle avec le double dispatch) est la même que celle que j'ai évoquée en solution 3. Mais elle ne me convient pas, du fait des arguments évoqués plus tôt dans mon premier post.

Enfin, il est vrai qu'on peut se passer de dynamic_cast dans bon nombre de cas, mais dans le présent cas j'ai juste l'impression qu'on ne fait que complexifier le code. Et surtout, la solution du double dispatch ne respecte pas le principe du SRP puisque, par exemple dans le cas du pattern Visitor, on est quand même obligé de rajouter une fonction supplémentaire dans la classe mère des classes visitées. Mais fondamentalement cette fonction n'a pas lieu d'être. D'un point de vue théorique on voit moins le problème mais en pratique... Voyons voir ce qui suit :

class Command
{
public:
    virtual std::string description() const;
 
    virtual void undo() = 0;
    virtual void redo() = 0;

    /**
     * Voici la function superflux dont je parle.
     * Elle n'a pas lieu d'être (d'un point de conceptuel) et ne sert que le double dispatch.
     */
    std::string getString(CommandStringifier &stringifier) {
        return stringifier.stringify(*this);
    }
};
 
class MacroCommand : public UndoCommand
{
private:
    std::vector<std::unique_ptr<UndoCommand>> m_subCommands;
 
public:
    void add(UndoCommand *command);
 
    std::string description() const override;

    void undo() override;
    void redo() override;
};

class CommandStringifier
{
public:
    virtual std::string stringify(const Command &cmd) = 0;
    virtual std::string stringify(const CommandMacro &macro) = 0;
};

main() {
    std::unique_ptr<Command> cmd( new ...);
    std::unique_ptr<CommandStringifier> stringifier(new ...);

    std::cout << strinifier.stringify(*cmd) << std::endl;  // KO (ne fera pas ce que l'on veut)
    std::cout << cmd->getString(stringifier) << std::endl; // OK
}

Comme tu peux le voir et conformément à ce que je disais dans mon précédent post, cela fonctionnerait mais c'est rajouter pleins de trucs à gauche et à droite juste pour éviter dynamic_cast dans les implémentations concrètes de Stringifier : c'est triste.  Personnellemment c'est une solution qui ne me séduit guère ; et c'est sans compter qu'il faudra utiliser ce même mécanisme de dispatch pour d'autres classes telles que CommandLocker, CommandPrinter, ... Et dans le cas où ce serait des utilisateurs autre que moi qui souhaiteraient rajouter ces classes, alors qu'il n'ont pas accès au source code (*.cpp), autant dire que c'est mision impossible (tout du moins inutilement complexifiée).

Toute autre argumentation ou solution serait la bienvenue.

misterFad a écrit:

Aussi, CommandSequence est bien une commande. Peut être est-ce le terme utilisé qui est trompeur mais par UndoSequence (ou CommandSequence), je designe tout simplement une macro commande : une commande qui est composée d'autres commandes.

Regarde du côté du pattern composite. Ce sera un peu moins crado que l'héritage direct.

misterFad a écrit:

Enfin, il est vrai qu'on peut se passer de dynamic_cast dans bon nombre de cas, mais dans le présent cas j'ai juste l'impression qu'on ne fait que complexifier le code. Et surtout, la solution du double dispatch ne respecte pas le principe du SRP puisque, par exemple dans le cas du pattern Visitor, on est quand même obligé de rajouter une fonction supplémentaire dans la classe mère des classes visitées.

Le double-dispatch permet principalement de rompre l'OCP de manière contrôlée. Le SRP n'est que très partiellement touchée pour une raison assez simple : sa capacité à interagir avec les autres membres de la hiérarchie par l'intermédiaire du visiteur est quelque part une partie de sa tâche.Le dynamic_cast rompt aussi l'OCP mais en plus il coûte la peau de noix à l'exécution, tu vas devoir écrire le dispatch à la main (ce qui est moche et source d'erreur).

En fait le problème ici est que tu sembles bosser sur quelque chose qui a manifestement trait à de la représentation de données d'AST (c'est peut être pas exactement ça, mais c'est exactement la même idée), et l'objet est très mal armé pour faire cela de manière pratique. Ici, on a des éléments variants dans une hiérarchie qui une fois qu'elle sera fixée aura peu, voir pas, de raison d'évoluer. Et ce qu'on utilise dans ce genre de cas, c'est un type algébrique. En C++ pour ça, il y a la notion de variant à partir de C++17.

Aussi, CommandSequence est bien une commande. Peut être est-ce le terme utilisé qui est trompeur mais par UndoSequence (ou CommandSequence), je designe tout simplement une macro commande : une commande qui est composée d'autres commandes. Ce que tu appelles donc CommandSequence dans ton post est ce que j'appelle UndoStack (ou CommandStack si tu veux). Je n'en ai pas parlé dans le post car cela n'était pas nécessaire.

Si j'ai fait une erreur d'interprétation en lisant le nom d'une de tes classes, dis toi que d'autres personnes lisant le même code risquent de faire la même erreur.  Tu es donc bel et bien face au symptôme qu'il y a un problème quelque part.  Et tu te dois d'y trouver une solution correcte.

N'oublie pas que "nommer c'est créer" ou, comme disait l'autre

-- Eragon (2006)

Brissingr signifie le feu.  C'est le feu.  Le mot, c'est la chose.  Connait le mot et tu maitrise la chose

En d'autres termes : choisir le bon nom pour représenter la chose que tu essaye de modéliser est d'une importance capitale, car c'est le seul moyen d'éviter toute ambiguïté pour les gens qui liront ton code

Tu croyais peut être ne jamais montrer ton code à personne. Et pourtant, tu en a écrit une partie sur le forum, ce qui est bien la preuve que tu te trompais sur le sujet.

Et même si tu n'avais jamais eu de question à poser au sujet de ton code, dis toi que tu auras toi-même très largement le temps d'oublier les détails de ton code au fur et à mesure de son évolution.

Si tu dois revenir sur ton code dans trois mois, six mois ou un an (ou peut-être même plus tôt!), tu auras oublié tellement de choses concernant les raisons de tes décisions actuelles que ce sera exactement comme si ce code avait été écrit par quelqu'un d'autre.

Si ta classe si ta classe CommandSequence est effectivement avant tout une commande, qui ne laissera pas la moindre ambiguité sur ce fait. 

Je ne sais pas, moi, choisi quelque chose comme "ScriptedCommand" ou "MultiCommand" ou, pourquoi pas CompositeCommand (du nom du DP composite qu'elle mettra à peu près en oeuvre) ...  Comme cela, quand tu reliras ton code dans un an, tu pourras te dire "ah oui: cette classe est bel et bien une commande qui peut en effectuer plusieurs"

Enfin, il est vrai qu'on peut se passer de dynamic_cast dans bon nombre de cas, mais dans le présent cas j'ai juste l'impression qu'on ne fait que complexifier le code. Et surtout, la solution du double dispatch ne respecte pas le principe du SRP puisque, par exemple dans le cas du pattern Visitor, on est quand même obligé de rajouter une fonction supplémentaire dans la classe mère des classes visitées. Mais fondamentalement cette fonction n'a pas lieu d'être. D'un point de vue théorique on voit moins le problème mais en pratique... Voyons voir ce qui suit :

class Command
{
public:
    virtual std::string description() const;
  
    virtual void undo() = 0;
    virtual void redo() = 0;
 
    /**
     * Voici la function superflux dont je parle.
     * Elle n'a pas lieu d'être (d'un point de conceptuel) et ne sert que le double dispatch.
     */
    std::string getString(CommandStringifier &stringifier) {
        return stringifier.stringify(*this);
    }
};
  
class MacroCommand : public UndoCommand
{
private:
    std::vector<std::unique_ptr<UndoCommand>> m_subCommands;
  
public:
    void add(UndoCommand *command);
  
    std::string description() const override;
 
    void undo() override;
    void redo() override;
};
 
class CommandStringifier
{
public:
    virtual std::string stringify(const Command &cmd) = 0;
    virtual std::string stringify(const CommandMacro &macro) = 0;
};
 
main() {
    std::unique_ptr<Command> cmd( new ...);
    std::unique_ptr<CommandStringifier> stringifier(new ...);
 
    std::cout << strinifier.stringify(*cmd) << std::endl;  // KO (ne fera pas ce que l'on veut)
    std::cout << cmd->getString(stringifier) << std::endl; // OK
}

 Si ce n'est que le code est faux: si tu veux profiter du polymorphisme (et du double dispatch par la même occasion),  la partie "visitable" du DP visitor doit... représenter un comportement polymorphe.

Et le seul moyen d'avoir un comportement polymorphe est d'avoir une fonction virtuelle.

Modifies seulement un truc dans ta classe de base pour lui donner une forme proche de

class Command
{
public:
    virtual std::string description() const;
  
    virtual void undo() = 0;
    virtual void redo() = 0;
 
    /* Je l'ai renommé, pour être plus précis
     * et j'ai juste rajouté la constante, mais ce doit
     * être une fonction virtuelle pure (à moins que tu
     * ne trouve un comportement adapté à utiliser quand on
     * ne sait pas de quel type est la commande... renvoyer une
     * chaine vide, par exemple ?)
     */
    virtual std::string strignify(CommandStringifier &stringifier) const = 0;
};

puis, redéfinis cette fonction dans toutes les classes dérivées, exactement comme tu l'auras fait avec undo et redo, par exemple

class MacroCommand : public UndoCommand
{
private:
    std::vector<std::unique_ptr<UndoCommand>> m_subCommands;
  
public:
    void add(UndoCommand *command);
  
    std::string description() const override;
 
    void undo() override;
    void redo() override;
    string stringnify(Stringifier & s) const{
        for(auto const & it : m_subCommands){
            it.get()->strignify(s);
        }
    }
};
/* et pour les commandes simples */
class MyCommand: public Command{
public:
    void undo() override{
        /* ... */
    }
    void redo() override{
        /* ... */
    }
    string stringnify(Stringifier & s) const{
        return s.visit(*this);
    }
}

Notes au passage que nous sommes en C++ et non dans un de ces foutus langages qui n'imaginent même pas que l'on puisse envisager de créer des fonctions libres.  Le patron visiteur, tel que décrit par le GoF et repris en coeur par tous les sites qui en parlent est basé sur une approche exclusivement orientée objet.  Mais C++ n'est pas exclusivement orienté objet.

Le paramètre est surtout là pour fournir un contexte qui peut parfaitement varier selon les situations, mais il peut tout aussi bien ne servir qu'à cela

Rien ne t'empêche -- car tu as raison sur un point : on passe notre temps à faire en sorte que notre code ignore complètement les différentes classes dérivée et ne connaisse que la classe de base, et on se retrouve à regrouper toutes les classes dérivées en un seul point, si l'on respecte scrupuleusement le DP visiteur (ou le DP factory, d'ailleurs) -- d'avoir recours à une fonction libre, ce qui réduira le couplage, sous une forme qui serait proche de

/* NOTE : cette fonction pourrait tout aussi bien ne pas 
 * être déclarée dans le fichier d'en-tête 
 */

void doStrignify(MacroCommand const & c, CommandStringifier & str){
   /* ce qui doit être fait quand on a spécifiquement affaire
    * à une MacroCommand
    */
}

et la redéfinission de strignify pour ta classe MaCroCommand prendrait alors la forme de

std::string MacroCommand::strignify(CommandStringifier & str) const{
    return doStrignify(*this, str);
}

En résumé, je te confirme que tu n'as jamais besoin du dynamic_cast. Et je viens de te le prouver

Quant à ta fonction main(), je te rappelles encore une fois que tu es en C++.  Ce n'est pas un langage qui impose d'avoir recours à l'allocation dynamique de la mémoire.  Au contraire: tu ne dois utiliser le recours à l'allocation dynamique de la mémoire que si tu n'as pas d'autre choix. 

Il y a fort à parier que ce soit le cas pour les commandes, mais, si on part du principe que CommandStringifier  est un visiteur polymorphe (elle serait alors bien mal nommée, mais soit), le type réel du visiteur dépend exclusivement du contexte dans lequel tu l'utilises; dans une portée qui lui est propre : dans une fonction X, ce pourrait être le visiteur concret A, et dans une fonction Y, le visiteur B, ou dans une alternative qui lui est propre.  Mais il n'a aucune raison de continuer à exister une fois que l'on sort de la portée dans laquelle il est utilisé.

Du coup, cette fonction pourrait très bien ressembler d'avantage à quelque chose comme (je complexifie un tout petit peu les choses, pour te permettre d'avoir une meilleure appréhension de la chose )

main() {
    /* OK... On utiliserais plutôt std::make_unique, mais soit
     */
    std::unique_ptr<Command> cmd( new ...);
    if(condition){
       ConcreteStrignifier1 cs;
       std::cout<<cmd.get->strignify(cs);
    }else{
        OtherConcreteStrignifer cs;
        auto result =cmd.get()->strignify(cs);
        /* on peut utiliser result ici */
    }
}

(tu auras compris que ConcreteStrignifier1 et OtherConcreteStrignifier dérivent toutes les deux de CommandStrignifier )

Enfin, je me rends compte que tu en es à un point de ta compréhension globale où les réponses "toutes faites" ne te suffisent plus.  C'est normal, et c'est une bonne chose, car tu sembles être (au moins sur ce point) ouvert à de plus amples explications.

Cette intervention est déjà trop longue (on va encore dire que j'ai encore frappé :P), et je ne vais donc pas entrer dans les détails de cette règle maintenant.

Mais si tu veux aller plus loin dans la compréhension de cette règle "immuable", fait le moi savoir, et je te donnerai tous les détails que tu peux souhaiter

Ksass`Peuk a écrit:

Regarde du côté du pattern composite. Ce sera un peu moins crado que l'héritage direct.

Tu fais bien de faire la remarque : effectivement on brise l'OCP. Mais rompre le SRP est tout aussi grave. Par exemple un appareil photo est capable de capturer un décor (objets, vivants, paysage, etc.) puis de l'encoder dans une image. Mais en aucun cas les éléments constitutifs du décor n'ont besoin de savoir ce qu'est un appareil photo, ni même d’être notifiés de son existence. De la même façon une commande n'a pas besoin de savoir ce qu'est une représentation textuelle du concept qu'elle définit, a fortiori si ladite représentation peut être déclinée sous plusieurs formes (et ce pour un même objet). Encore une fois, j'ai l'impression que opter pour le double dispatch n'est pas tant motivé par une convergence vers une solution fonctionnelle mais plutôt par l'envie d'éviter dynamic_cast. Et çà c’est triste.

Ksass`Peuk a écrit:

En fait le problème ici est que tu sembles bosser sur quelque chose qui a manifestement trait à de la représentation de données d'AST (c'est peut être pas exactement ça, mais c'est exactement la même idée), et l'objet est très mal armé pour faire cela de manière pratique. Ici, on a des éléments variants dans une hiérarchie qui une fois qu'elle sera fixée aura peu, voir pas, de raison d'évoluer. Et ce qu'on utilise dans ce genre de cas, c'est un type algébrique. En C++ pour ça, il y a la notion de variant à partir de C++17.

Effectivement tu as raison. Cela me rappelle un peu un compilateur que j’ai écrit récemment. A un moment il fallait parcourir l’AST afin de détecter les éventuelles erreurs sémantiques. Au final j’ai fini avec 36000 visiteurs (en Java mais on aurait eu le même problème avec C++ ou autres). Par contre avec OCAML le code était beaucoup plus compact, du fait notamment du pattern matching.

Au passage, la notion de std::variant dont tu parles m’a l’air très intéressante mais forcer le support de C++17 n’est pas vraiment souhaitable. Le code ne supportera toutefois pas des versions antérieures à C++11.

@koala01: Pour le nommage tu as entièrement raison. Il faut dire que mon choix a été légèrement influencé par ceci. Mais je reverrais tout ça.

koala01 a écrit:

Tu croyais peut être ne jamais montrer ton code à personne. Et pourtant, tu en a écrit une partie sur le forum, ce qui est bien la preuve que tu te trompais sur le sujet. Et même si tu n'avais jamais eu de question à poser au sujet de ton code, dis toi que tu auras toi-même très largement le temps d'oublier les détails de ton code au fur et à mesure de son évolution.

Ma conclusion ?

Enfin je vous remercie pour vos réponses : je demeure néanmoins sur ma faim.

misterFad a écrit:

Tu fais bien de faire la remarque : effectivement on brise l'OCP. Mais rompre le SRP est tout aussi grave.

Sauf que pouvoir être visité, c'est bien une opération qu'on attend d'un AST amha. Si c'est le fait de devoir créer différentes fonctions supplémentaires qui t'inquiète, tu peux te contenter de définir une notion abstraite de visiteur qui sera spécialisée en fonction de la tâche à réaliser (voir la page wikipedia anglaise du DP visitor).

misterFad a écrit:

Au passage, la notion de std::variant dont tu parles m’a l’air très intéressante mais forcer le support de C++17 n’est pas vraiment souhaitable. Le code ne supportera toutefois pas des versions antérieures à C++11.

Il y a boost::variant, qui est header only je crois.

misterFad a écrit:

Ton raisonnement est cohérent mais part du postulat que la fonction stringify est légitime pour la classe Command. Mais ce n'est absolument pas le cas.

C'est là qu'entrent en jeu la notion d'interface et l'ISP (Interface Segregation Principle ou principe de ségrégation des interfaces)

Si une partie de l'interface n'est pas légitime pour l'ensemble des classes dérivées, extrait la de ta classe de base et crées en une autre classe dont "n'importe qui" pourra hériter

En java et en C#, on te dirait d'utiliser les mots clés interface et implements; et tu subirais la restriction (due à l'utilisation du ramasse miettes) que cette interface ne peut pas contenir la moindre donnée membre, et que toutes les fonctions soient virtuelle pures

Ces restrictions et cette distinction n'existent pas officiellement en C++, mais tu es toujours en mesure d'éviter qu'une classe ne soit instanciée autrement que par les classes dérivées (par exemple, en placant le constructeur et un destructeur non virtuel dans l'accessibilité protégée)

Et, bien sur, le principe est valable pour toute partie de l'interface qui n'est nécessaire qu'à un "certain niveau" ta hiérarchie de classe

Une autre solution qui serait à mon avis plus correcte serait de procéder à la Java (avec une fonction toString() et donc retirer le Stringifier). Mais cela limiterait le mécanisme de stringifying à celui défini par la classe : soit on conserve ce qui existe, soit on le redéfinit (auquel cas il faudra créer une nouvelle classe fille).

En effet. Certains choix doivent être faits. Mais veilles à te poser toutes les questions nécessaires.

Et puis, il arrive un moment où il faut écrire un minimum de code, aussi

De toutes les solutions (dynamic_cast exclu) la solution du double dispatch apparaît être la plus adaptée. Mais pour autant, je ne peux que vaguement justifier pourquoi cette solution est préférée à dynamic_cast,<snip>

Nous voici donc parvenusla deuxième étape : introduire les exceptions à la règle afin de t'aider à mieux appréhender la règle.

La première chose dont il faut avoir conscience, c'est que les principes de conception priment tout, ce qu'un langage peut permettre ne les remettra pas en question.

Les cinq grands principes à respecter sont connu sous l'acronyme SOLID et parmi ceux-ci, seul le L (pour LSP) est spécifique à tout ce qui a trait à l'orienté objet et à l'héritage.

Celui qui nous intéresse particulièrement est le O pour OCP qui est l'acronyme de Open Close principle, ou, si tu préfères en français, le principe "ouvert / fermé".

Ce principe nous dit que le code doit être fermé aux modifications mais ouvert aux évolutions.  Autrement dit, une fois que tu as vérifié et validé un comportement (celui d'une fonction, par exemple), tu ne devrais plus avoir besoin de le modifier pour pouvoir introduire une évolution.

Il se peut, bien sur, que tu doives le modifier pour corriger un bug ou pour améliorer l'algorithme, mais ça tient d'avantage de la maintenance que du développement et de la conception ;).  Or, ici, on parle... de conception et de développement.  Et voici (enfin) ce qu'il faut savoir:

static_cast et dynamic_cast (les différences entre les deux sont suffisamment minimes que pour qu'on les étudie en même temps )permettent de forcer le compilateur à considérer une donnée qu'il n'est sensé connaitre que comme étant du type de base comme une donnée d'un type dérivé clairement identifié.  Chouette!  Il est vrai que, dans certains cas, cela peut s'avérer vachement utile!

Mais, si on l'utilise à mauvais escient, ca va rapidement virer au cauchemar.

Le gros risque est d'en arriver à utiliser dynamic_cast pour assurer une certaine forme de RTTI qui pourrait ressembler à quelque chose comme

void foo(Base * b){
    if(dynamic_cast<Derivee1*>(b)){
        /* un truc spécifique à Derivee1 */
    }
    else if(dynamic_cast<Derivee2*>(b)){
        /* un truc spécifique à Derivee2 */
    }
    else if(dynamic_cast<Derivee3*>(b)){
        /* un truc spécifique à Derivee3 */
    }
    /* ... */
}
/* ca pourrait aussi être */
void bar(Base * b){
    if(b-<typeInfo()== SpecifiqueADerivee1){
        static_cast<Derivee1>(b)->specificToDerivee1();
    }
    else if(b-<typeInfo()== SpecifiqueADerivee2){
        static_cast<Derive2>(b)->specificToDerivee2();
    }
    else if(b-<typeInfo()== SpecifiqueADerivee3){
        static_cast<Derivee3>(b)->specificToDerivee3();
    }
    /* ... */
}

Pourquoi? parce que ca brise l'OCP: Si, à un moment donné, tu crées une nouvelle classe dérivée (mettons: DeriveeN), tu devras modifier tous les endroits du code où une telle pratique aura été mise en oeuvre.

Le truc, c'est que tu auras trouvé cette manoeuvre "tellement utile" que tu retrouveras ce genre de code à peu près partout.  Et la loi de finagle aidant, lorsque tu voudra ajouter une nouvelle classe dérivée, tu dois t'attendre à forcément oublier un des endroits du code ou elle apparait.

Cela se traduira par un résultat prévisible : un bug, qu'il faudra bien corriger plus tard (au risque d'introduire d'autres bugs).

Alors, si tu n'a qu'une seule possibilité de transtypage (comprends: que tu n'as aucun test à faire pour savoir que tu transtype vers le bon type), sous une forme qui serait proche de

void foo(Base * b){
    /* je suis dans une situation dans laquelle je SAIS
     * PERTINEMMENT que j'ai affaire à un élément qui est
     * de type Derivee1, et j'ai besoin d'un de ses 
     * caractéristiques propres
     *
     * je peux transtyper "sans risque"
     */
   Derivee1 * ptr = static_cast<Derivee1 *>(b);
   /* utilisation de ptr en tant que Derivee1 */
}

Mais si tu n'es pas dans ce cas tout particulier, le transtypage, tu oublies, parce que cela ne fera que poser des pièges dans lesquels tu ne manquera pas de tomber plus tard.

Est ce que cela te convainc un peu plus?

-
Edité par koala01 20 septembre 2017 à 20:19:25

@Ksass`Peuk:

Concernant boost::variant, l'utiliser constituerait en fait en le rajout d'une dépendance pas nécessaire avec boost. Je ferais avec le double dispatch.

Quant au visiteur, effectivement le fait de devoir rajouter des fonctions qui ne devraient pas exister (d'un point de vue métier) me rebute un peu. Je pense notamment à la fonction File::accept de cet exemple de Wikipedia. Mais quand on y pense, on a quand même besoin d'une telle fonction car sinon, il serait impossible d'opérer sur une classe concrète alors qu'on ne dispose que d'un pointeur ou d'une référence sur une classe parent. Et quand bien même on ferait avec la solution du pattern Visitor, une fois l'architecture fixée, toute personne souhaitant visiter de nouvelles classes devra modifier l'interface du Visiteur (on brise bien le OCP comme tu le disais). Et ça, c'est en supposant que ladite personne a accès au code source, sinon bienvenue la galère.

On en vient alors au commentaire de koala01

Nous voici donc parvenus à la deuxième étape : introduire les exceptions à la règle afin de t'aider à mieux appréhender la règle.

Bien sûr, une fois l'exception introduite, le problème est résolu. Imagines un peu ton chirurgien dentiste t'arracher une dent à chaque fois que tu le consultes pour des douleurs au niveau de la denture. Autrement dit dès lors qu'on s'autorise à briser le SRP, forcément tout devient plus simple à gérer. Mais je ne pense qu'on n'y peut trop rien, nos langages de programmation ne nous aidant pa plus que ça. Et je suis entièrement d'accord avec ton argumentation concernant dynamic_cast et le fait qu'on brise l'OCP. Cela dit, est-il plus légitime de s'autoriser à briser un principe plutôt qu'un autre ? Je pense que non mais on peut toujours s'autoriser à faire des choix, comme tu le dis.

koala01 a écrit

Est ce que cela te convainc un peu plus?

Convaincu ? Non. Mais il est clair que l'argumentation est beaucoup plus satisfaisante. Et c'est suffisant je pense.

bacelar a écrit:

Enfin, moi, ce que je vois, c'est un mécanisme de sérialisation en chaine de caractère de commandes bricolées, et dans ce cas, bin dynamic_cast il marche pas. Débat clos.

Heureusement que ce n'est que ton point de vu. Imagines Bob se présenter dans le bureau de son patron pour solliciter une augmentation (pour une raison ou une autre). Et là le patron lui répond : "... et bien ce n'est pas mon problème". Bin dans ce cas, ça ne passe pas non plus.

Mais je te rassure, non seulement la fonctionnalité est légitime, mais en plus le problème qu'on essaye de résoudre ici n'est pas inhérent à mon projet. C'est un problème récurrent qui a été solutionné depuis plusieurs années maintenant. Il m'apparaît tout simplement que ces solutions ne respectent pas non plus SOLID, qui, je le rappelle, est supposé être le piler de toute architecture orientée objet bien construite... Enfin bon, on s'égare un peu là.

misterFad a écrit:

Concernant boost::variant, l'utiliser constituerait en fait en le rajout d'une dépendance pas nécessaire avec boost.

Header-only, ça veut dire que tu prends les headers, tu les ajoutes dans ton dossier include et c'est terminé. Personne ne paiera la dépendance.

misterFad a écrit:

Bien sûr, une fois l'exception introduite, le problème est résolu. Imagines un peu ton chirurgien dentiste t'arracher une dent à chaque fois que tu le consultes pour des douleurs au niveau de la denture. Autrement dit dès lors qu'on s'autorise à briser le SRP, forcément tout devient plus simple à gérer. Mais je ne pense qu'on n'y peut trop rien, nos langages de programmation ne nous aidant pa plus que ça. Et je suis entièrement d'accord avec ton argumentation concernant dynamic_cast et le fait qu'on brise l'OCP. Cela dit, est-il plus légitime de s'autoriser à briser un principe plutôt qu'un autre ? Je pense que non mais on peut toujours s'autoriser à faire des choix, comme tu le dis.

Ton analogie est quelque peu foireuse, car elle irait plutôt dans l'autre sens : la règle générale étant de se contenter d'un détartrage quoi qu'il arrive, avant d'envisager des plombages, arrachages de dents et autres soins de chirugie dentaire

De plus, il ne s'agit jamais de briser le SRP : chaque donnée, chaque type de donnée, chaque fonction ne doit s'occuper que d'une et une seule chose pour que l'on puisse s'assurer qu'elle s'en occupe correctement.

Par contre, l'OCP est le seul principe à introduire une notion sur la manière d'implémenter les fonctions : une fois le code écrit (et validé), tu ne devrais avoir aucune raison de le changer.

Le transtypage (quel qu'il soit) tend exclusivement à briser l'OCP, et il n'est absolument pas question de choisir entre respecter le SRP ou respecter l'OCP.  Tu peux, bien sur, toujours arriver à des situations où tu respectes l'un mais pas l'autre. Mais ces situations ne font que mettre un défaut de conception en évidence.  Si bien que l'idéal est donc toujours de respecter les deux, 

En fait, sur les cinq principes SOLID, tu as deux principes transversaux qui sont SRP et OCP, un principe spécifique à l'orienté objet (et à la substituabilité que le paradigme permet) qui est le LSP et les deux derniers (ISP et DIP) ne font que donner des pistes susceptibles de te mener à un meilleur respect des deux premiers.

A tel point que si tu repecte à la lettre le SRP et l'OCP, il est plus que probable que tu en arriveras forcément à respecter également l'ISP et le DIP.  Et l'autre point de vue est de se dire que si tu respecte scrupuleusement ISP et DIP, tu as déjà fait le gros du travail pour atteindre un respect ad minima satisfaisant du SRP et du DIP.

Convaincu ? Non. Mais il est clair que l'argumentation est beaucoup plus satisfaisante. Et c'est suffisant je pense.

C'est sans doute parce que tu es encore en "phase de transition", dans le sens où l'explication t'a satisfait (c'est toi-même qui l'as dit, hein?), mais où tu n'es toujours pas forcément convaincu ni de l'impérative nécessité de respecter SOLID ni de la possibilité qu'il y a à respecter les cinq principes sans contrevenir à aucun d'entre eux

Je peux te garantir que, non seulement, il y a moyen de respecter scrupuleusement ces cinq principe (ainsi que la loi de Déméter, tant qu'à faire), mais qu'en plus, c'est le seul moyen de garantir que tu ne prépares pas aujourd'hui des pièges dans lesquels tu tomberas demain, en perdant bien plus de temps que les cinq minutes de réflexion que tu t'accordes aujourd'hui

bacelar a écrit:

Heureusement que ce n'est que ton point de vu. Imagines Bob se présenter dans le bureau de son patron pour solliciter une augmentation (pour une raison ou une autre). Et là le patron lui répond : "... et bien ce n'est pas mon problème". Bin dans ce cas, ça ne passe pas non plus.

Les analogies, c'est bien, mais il faut comparer ce qui est comparable. On peut (je le fais souvent) parfois "personnifier" l'ordinateur ou le compilateur afin de faire "comme si" on s'adressait à un humain

Mais il ne faut jamais oublier qu'un ordinateur ou un programme (et un compilateur n'est rien d'autre qu'un programme, on est d'accord) n'a aucune intelligence propre. Il respecte les règles qu'on lui impose.

Comme je le dis régulièrement, un programme ou un ordinateur est un "brave petit soldat" qui ne fera jamais preuve de la moindre initiative: si on lui dit de sauter, il saute, s'inquiéter de savoir s'il est intéressant de sauter maintenant ou d'attendre un peu avant de le faire, et sans se poser la question de la hauteur ou de la distance du saut. Il sautera toujours aussi loin et aussi haut qu'il peut. Point barre

Si tu commences à comparer un ordinateur (qui ne prend aucune décision de lui-même) à un décideur, tu ne peux pas faire d'avantage fausse route

Mais je te rassure, non seulement la fonctionnalité est légitime, mais en plus le problème qu'on essaye de résoudre ici n'est pas inhérent à mon projet. C'est un problème récurrent qui a été solutionné depuis plusieurs années maintenant. Il m'apparaît tout simplement que ces solutions ne respectent pas non plus SOLID, qui, je le rappelle, est supposé être le piler de toute architecture orientée objet bien construite... Enfin bon, on s'égare un peu là.

Effectivement, SOLID est sensé être le pilier de tout programme informatique bien construit. Et, à part le L qui est spécifique à l'OO, les autres s'appliquent également dans les autres paradigmes.

Il faut cependant aussi prendre conscience que la technologie de l'informatique telle que nous la connaissons est sommes toutes très récente: son histoire commence en 1945 avec Von Neumann et son rapport sur l'EDVAC

Quant au premiers langages de programmation dit "de troisième génération" (dont font partie tous les langages actuels), ils ne sont apparus qu'en 1954 avec le FORTRAN. Si bien que les sciences informatiques telles que nous les connaissons aujourd'hui n'ont à peu près qu'une grosse soixantaine d'années.

Penses, pour ne parler que de langages que tu connais au moins de nom, que le développement originel de C a été entrepris de 1969 à 1973, que C++ n'est apparu qu'en  1983 (première norme officielle en 1998), que java est apparu en 1995 et que C# est apparu en ... 2000.

Les principes SOLID? je ne crois pas en avoir entendu parler avant 2003 ou 2005 alors que je m'intéressait déjà à la programmation (en générale) et au C++ (en particulier) depuis plusieurs années...

Tout cela pour te dire que les sciences informatiques en générale sont des sciences particulièrement jeunes et, selon toute vraisemblance, encore en pleine évolution.

Alors, oui, bien sur, les problèmes de l'analyse syntaxique et lexicale ont été abordés et implémentés bien avant l'arrivée des principes SOLID.  Et c'est normal : nous n'aurions rien pu faire sans eux. Et, oui, tu as raison : la théorie concernant ces problèmes n'a guère évolué depuis qu'elle a été exprimée (sans le recours aux principes SOLID).

Mais, si ta question est "pourquoi se faire ch...er à essayer de respecter SOLID dans ce cas bien particulier, alors qu'on s'en passe si bien depuis plus de cinquante ans?", la réponse est : parce que cela pourra te simplifier la vie (quoi que tu en penses), en t'évitant de dresser des pièges dans lesquels tu seras le premier à tomber.

Seulement, pour te convaincre de cela, il n'y aura que l'expérience, dont tu sembles cruellement manquer à l'heure actuelle. 

Mais, si quelqu'un qui a largement plus d'expérience que toi -- ne serait-ce que pour avoir participer à plusieurs projets composés de plusieurs milliers de fichiers représentant plusieurs centaines de milliers de lignes de code (si pas plusieurs millions de lignes de code) --  te dis que c'est le seul moyen d'éviter de tomber dans des pièges que tu auras toi-même dressé, je crois sincèrement que tu peux partir du principe qu'il sait de quoi il parle et que son seul but est de t'éviter de reproduire des erreurs qui ont déjà été reproduites bien trop souvent depuis qu'elles ont été clairement identifiées.

-
Edité par koala01 21 septembre 2017 à 0:26:51

koala01 a écrit:

Ton analogie est quelque peu foireuse, car elle irait plutôt dans l'autre sens : la règle générale étant de se contenter d'un détartrage quoi qu'il arrive, avant d'envisager des plombages, arrachages de dents et autres soins de chirugie dentaire

Non, même pas, l’analogie est bien correcte : on introduit bien une exception (cf. arrachage de dents) et une fois celle-ci acceptée, forcément le problème à résoudre n’est plus. Mais vu que tu sembles ne pas être d’accord avec le fait que certaines pratiques supposées « bonnes pratiques de la programmation » reposent en fait sur ce principe de discrimination, je te propose l’exemple ci-après.

Considérons un système où on a juste besoin de modéliser

• des bicyclettes et des voitures
• ainsi que des humains pour les conduire

class Vehicle {};
class Bicycle : public Vehicle {};
class Car :     public Vehicle {};

class Human { // temporaire (on verra pourquoi)
public:
    virtual void drive(Vehicle &vehicle) = 0;
};

class Citizen : public Human { // temporaire aussi (puisque Human l’est)
private:
    // name, age, etc.

public:
    void drive(Vehicle &vehicle) override { /* … */ }
};

// On écrit plutôt deux fonctions drive

class Human {
public:
    virtual void drive(Bicycle &bicycle) = 0;
    virtual void drive(Car &car) = 0;
};

class Citizen : public Human {
private:
    // name, age, etc.

public:
    void drive(Bicycle &bicycle) override { /* … */ }
    void drive(Car &car)         override { /* … */ }
};

main() {
    Bicycle bicycle;
    Car car;

    Citizen c;
    Human &h = c; // juste pour éviter new/delete/unique_ptr

    h.drive(bicycle); // OK
    h.drive(car);     // OK
}

class StylizedBicycle : public Bicycle {
    // ...
};

// on rajoute une nouvelle fonction

class Human {
public:
    virtual void drive(Bicycle &bicycle) = 0;
    virtual void drive(StylizedBicycle &bicycle) = 0; // à implémenter aussi dans Citizen
    virtual void drive(Car &car) = 0;
};

main() {
    Bicycle b1;
    StylizedBicycle b2;

    Citizen c;
    Human &h = c; // juste pour éviter new/delete/unique_ptr

    h.drive(b1); // OK
    h.drive(b2); // OK

    Bicycle* bicycles[] = {&b1, &b2};
    for(auto *b : bicycles) {
        h.drive(*b); // OK MAIS FOIREUX
    }
}

virtual Bycicle::drivenBy(Human &human) {
    human.drive(*this); // OK car *this est bien une Bicycle
}

StylizedBycicle::drivenBy(Human &human) override {
    human.drive(*this); // exactement le même code
                        // OK car *this est bien une StylizedBycicle
}

C’est moche. C’est encore plus moche puisqu’on conserve les fonctions drive de Human mais on ne peut s’en servir au risque de voir resurgir le problème ci-dessus évoqué. En C++ on pourrait toujours rendre ces fonctions private/protected quitte à rajouter un lien d’amitié (friend). Mais cela reste du bricolage. Et c’est sans mentionner le fait qu’on brise clairement le SRP puisqu’en réalité la fonction rajoutée ne sert aucunement la classe Bycicle. Elle permet juste à Human de bien faire son job. C’est une fonction qui, conceptuellement parlant, n’a pas lieu d’être.

koala01 a écrit:

C'est sans doute parce que tu es encore en "phase de transition", dans le sens où l'explication t'a satisfait (c'est toi-même qui l'as dit, hein?), mais où tu n'es toujours pas forcément convaincu ni de l'impérative nécessité de respecter SOLID ni de la possibilité qu'il y a à respecter les cinq principes sans contrevenir à aucun d'entre eux

Le problème n'est clairement pas là. J’ai découvert SOLID tout récemment et je dois avouer que je trouve le principe plutôt rassurant. C’est plutôt le double dispatch (cf. pattern Visitor) qui pose ici problème. A mon avis et au vu des arguments exposés dans ce post, s’autoriser à appliquer le double dispatch (et donc Visitor par extension), c’est s’autoriser à enfreindre SOLID. Mais effectivement une autre façon de conclure serait de mettre en tort ou SOLID ou la (supposée bonne) pratique de programmation. Personnellement je ne tirerais aucune de ces conclusions, tout simplement parce que ton argumentation concernant l’émergence des langages de programmation (concept, syntaxe, etc.) tient la route. Et au passage, je n'ai pas dit que j'étais satisfait : j'ai dit que l'argumentation était beaucoup plus satisfaisante, et ce, moyennant bien sûr l'acceptation de l'hypothèse comme quoi une exception a été introduite.

koala01 a écrit:

Seulement, pour te convaincre de cela, il n'y aura que l'expérience, dont tu sembles cruellement manquer à l'heure actuelle.

Il est clair que c’est en forgeant qu’on devient forgeron. Pour autant les arguments que j’évoque plus haut n’ont a priori rien à voir avec l’expérience. Il est plutôt question de constatation/observation. Cela dit, je pense que c’est la syntaxe proposée par nos langages de programmation qui me donne cette impression de bricolage. Je ne peux donc que faire avec.

koala01 a écrit:

Mais, si quelqu'un qui a largement plus d'expérience que toi -- ne serait-ce que pour avoir participer à plusieurs projets composés de plusieurs milliers de fichiers représentant plusieurs centaines de milliers de lignes de code (si pas plusieurs millions de lignes de code) -- te dis que c'est le seul moyen d'éviter de tomber dans des pièges que tu auras toi-même dressé, je crois sincèrement que tu peux partir du principe qu'il sait de quoi il parle et que son seul but est de t'éviter de reproduire des erreurs qui ont déjà été reproduites bien trop souvent depuis qu'elles ont été clairement identifiées.

Le fait que tu ne puisses me convaincre ne remet aucunement en cause ta supériorité en matière de programmation. Dans ton précédent post tu faisais allusion aux années 2000. Moi en 2000 j'étais encore à l'école primaire primaire. Mais ce n'est pas pour autant que je me forcerais à être systématiquement toujours en accord avec ce que disent les aînés. Bien entendu, loin de moi l'idée de remettre en cause ce qui a été établi (du moins pas pour l'instant, vu que j'ai encore beaucoup à apprendre). Mais il est clair qu'il y a un truc qui ne va pas au sein même du principe de la programmation. Mais comme je le disais plus tôt, on n'y peut rien, vu les outils que nous proposent nos langages de programmation.

A vrai dire, ce qui ne va pas, c'est que tu n'implémente pas correctement le patron visiteur. Car tu as oublié une surcharge de fonction primordiale dans ton visiteur (dans ta classe Human en fait): la surcharge qui permet d'invoquer drive sur... un véhicule (sans autre précision).

Etant donné que, pour pouvoir maintenir le polymorphisme, tu ne pourras maintenir plusieurs véhicules (sans plus de précision) dans une collection que sous la forme de pointeurs sur le véhicule, cette surcharge devrait prendre un ... Vehicle *.  Laisse moi te montrer (je reprends ici ton propre code pour une très grosse partie):

class Human {
public:
    /* Virtuelle, ou pas? à voir si le fait d'être conduit par
     * un citoyen au lieu d'un humain fait la différence
     */
    void drive(Vehicle * v){
        v->drivenBy(*this);
    }
    virtual void drive(Bicycle &bicycle) = 0;
    virtual void drive(Car &car) = 0;
};
 
class Citizen : public Human {
private:
    // name, age, etc.
 
public:
    void drive(Bicycle &bicycle) override { /* … */ }
    void drive(Car &car)         override { /* … */ }
};
main() {
    Bicycle b1;
    StylizedBicycle b2;
 
    Citizen c;
    Human &h = c; // juste pour éviter new/delete/unique_ptr
 
    h.drive(b1); // OK
    h.drive(b2); // OK
 
    Bicycle* bicycles[] = {&b1, &b2};
    for(auto *b : bicycles) {
        h.drive(b); // OK car initialise le double dispatch
    }
}

 (le seul truc dans ta conception, c'est que je ne suis pas sur qu'un citoyen conduise n'importe quel type de véhicule d'une manière différente d'une personne... Mais bon, ce n'est pas vraiment un problème )

Avec cette surcharge "toute bête" supplémentaire, ton problème disparait purement et simplement.  Magique, non?

-
Edité par koala01 21 septembre 2017 à 22:32:37

J'ai vraiment l'impression que j'ai du mal à me faire claire. Pourtant je ne vois pas ce qu'il y a de si obscure dans mes propos. L'implémentation que tu proposes ne change absolument rien, puisqu'on maintient la fonction drivenBy. On tourne un peu en rond là, alors que le but se trouve juste sous notre nez. De même, séparer (ou non) Human de Citizen n'est pas non plus un problème. Et ton code remonte en fait le problème au niveau de Vehicle alors que j'ai fait exprès de le laisser au niveau de Bicycle pour montrer que le problème n'intervient que lorsqu'une même fonction est surchargée comme dans le cas ci-après :

class Parent {};
class Child : public Parent {};

class SomeClass {
    void doItFor(Parent &) {} // voici les
    void doItFor(Child &) {}  // fonctions "problème"
};

Parent p;
Child c;

SomeClass s;
Parent* parents = {&p, &c};
s.doItFor(*parents[0]);
s.doItFor(*parents[1]);

Peu importe qu'on ait des fonctions virtuelles ou non, on est mal puisque seule une version de doItFor sera appelée dans tous les cas. Ce n'est sans doute pas la fin du monde mais dans un cadre d'utilisation concret (comme c'est le cas pour UndoStringifier par exemple), forcément le workaround utilisé reposera sur le double dispatch, qui, implicitement, brise SOLID (cf. mon argumentation précédente). Et ce, tout simplement par ce nos langages de programmation ne permettent pas de faire autrement. On introduit donc bien une exception mais il se trouve juste qu'on n'y peut rien.

Et moi, je t'explique que le double dispatch, lorsqu'il est correctement mis en oeuvre, ne souffre d'aucun problème.  Ou, plus précisément que je n'arrive pas à comprendre en quoi tu estimes qu'un problème pourrait se poser avec lui.

Entre le fait qu'une donnée connait forcément son type le plus dérivé, la virtualité des fonctions et leur surcharge, tu n'as absolument besoin de rien d'autre pour arriver à traiter tous les cas dans lesquels tu pourrais être tenté d'avoir recours à une forme ou une autre de RTTI (Run Time Type Information).

La seule exception que je pourrais trouver serait celle où l'on utilise le type erasure tout en fournissant une fonction (template?) qui nous permettrait de récupérer le type d'origine.  le RTTI permettant à ce moment là de fournir la garantie que la donnée peut être convertie dans le type indiqué et le transtypage permettant de le faire de manière effective (un peu à la manière de boost::any et de any_cast).

Maintenant, je peux comprendre que le double dispatch puisse sembler disproportionné, inutilement complexe ou vachement circonvolué par rapport à certains problèmes, mais je peux t'assurer que c'est pourtant la solution qui te posera le moins de problèmes à moyen ou à long termes.

Car toutes les alternatives au double dispatch poseront des problèmes au moment où tu voudras commencer à insérer des évolutions.  Et ces problèmes évolueront de manière exponentielle à chaque évolution ajoutée depuis que tu auras pris la décision d'utiliser une des alternatives possibles.

Tu ne vois peut-être pas les problèmes que posent les alternatives au double dispatch lorsque tu n'as -- en gros --  qu'une classe de base et  X classes dérivées qui ne sont utilisées qu'à un seul endroit.  A ce moment là, je peux parfaitement comprendre que tu te demandes pourquoi se faire du mal inutilement (car c'est effectivement à cela que ressemble le double dispatch).

Mais tu dois te dire que ton projet va évoluer avec le temps.  Et comme "l'alternative au double dispatch aura tellement bien fonctionné" la première fois que tu l'auras mise en oeuvre, tu seras en toute logique tellement bien disposé par rapport à cette pratique que tu la reproduira à un endroit, puis à un autre, et encore un autre.

Puis, un jour, tu voudras rajouter une fonctionnalité, et tu devras rajouter une classe dérivée.  Avec un peu de chance, tu te souviendras du dernier endroit où tu  as utilisé le RTTI pour déterminer le type réel de ta donnée, et tu pensera à le modifier.   Avec beaucoup de chance, tu te souviendras aussi de l'avant dernier endroit ou tu as utilisé cette technique.

Mais combien vas tu en oublier?  Combien de bug est-ce que cela va provoquer?  Combien de temps te faudra-t-il avant de te rendre compte de l'existence de ces bugs?  Et combien de temps te faudra-t-il pour les corriger?

Car il faut aussi comprendre que, plus une erreur est repérée tôt après qu'elle ait été commise, plus il est facile de la corriger.  Ainsi, si tu fais une erreur de syntaxe dans ton code, tu t'en rendras compte lors de la compilation suivante; et comme tu n'auras modifié qu'un nombre restreint de lignes de code entre deux compilation (et que, en plus, le compilateur pousse la gentillesse jusqu'à te dire le fichier et la ligne où l'erreur se trouve), tu pourra la corriger très rapidement.

Après cette étape, on en vient au tests unitaires et tests d'intégration.  Quand une erreur survient lors de ces tests, elle est déjà plus compliquée à situer, car elle peut tout aussi bien trouver son origine dans la fonction où elle apparait que... dans une des fonctions qui ont été appelées pour y arriver.

Encore après, il y a les bugs qui sont remontés de la version RC ou de la version en production.  Certains bugs apparaissent des mois voir des années après que le code qui en est responsable n'ait été écrit.  Il arrive même que la dernière personne à avoir posé les yeux sur le code ait quitté la boite! 

Et puis, il y a eu tant de code écrit et modifié depuis que le code qui contient l'erreur a été écrit; il y a tant de code qui utilise l'erreur et qui fournit malgré tout un résultat jugé correct et cohérent, qu'il devient particulièrement difficile de situer l'erreur dans le code.  Mais il devient encore plus difficile de le corriger, parce que cela occasionne quantité de nouvelles erreurs au niveau du code qui utilise la partie en erreur.

Et je peux t'assurer que, dans ces moments là, tu commence à t'arracher les cheveux .

Tu veux un exemple de ce que j'essaye de t'expliquer?  Il y a quelques années, on s'est rendu compte que l'implémentation d'un algorithme de création de clé de hashage pour la signature numérique de documents était erronée (était-ce le RSA?).  Pas de bol, ce système de signature était utilisé de manière massive, et toutes les clé générée avec cette implémentation erronée  semblaient correcte.  Mais aucune ne l'aurait été en utilisant l'implémentation correcte de l'algorithme.

Si bien que la "petite correction" (si mes souvenir sont bons, ca consistait à remplacer un XOR binaire par un OU binaire, ou quelque chose du genre) a nécessité de mettre quantité de clé et un nombre incalculable d'applications à jour.  Petites causes, très gros effets.

Et je pourrais aussi te parler du bug de l'an 2000, qui n'est heureusement jamais arrivé, mais qui a nécessité plusieurs centaines de milliards de dollars pour mettre tous les systèmes à jour afin de l'éviter pour arriver à te convaincre de des choix qui peuvent sembler judicieux à un instant T peuvent s'avérer catastrophiques sur le long terme

Je n'ai nulle part mentionné dans mon post original que j'allais utilisé dynamic_cast. Je disais bien que la solution qui paraissait la plus viable repose sur un concept connu sous le nom de double dispatch. Mais s'autoriser à l'utiliser revient en fait à briser SOLID et ça, je le démontre bien dans ce post. C'est la raison pour laquelle je disais que les deux alternatives étant erronées (vis-à-vis de SOLID), il n'y en a pas une qui est meilleure que l'autre (simple constat). Mais bien entendu, moyennant explications diverses et variées, on peut toujours s'autoriser à choisir l'une plutôt que l'autre.

J'adore la programmation (réfléchir et trouver une solution), j'adore le travail bien fait (du moins je m'efforce de plus en plus), mais cela ne veut pas dire que je dois fermer les yeux lorsque c'est moche. Dans tous les cas merci à tous (et à toi en particulier).

Le fait est que tu ne démontres rien à part... ton manque d'habitude.

Car, bien que l'on parle très souvent des principes SOLID de manière séparée, le "bon" développeur -- allez, pour ne froisser personne, disons plutôt le développeur expérimenté -- va arriver à les appréhender de manière "intégrée" et saura que la seule manière correcte de s'y prendre est de respecter scrupuleusement les cinq principes en question (ainsi que la loi de Déméter, d'ailleurs).

Et la cause principale de ton souhait vient fait que tu n'appréhendes sans doute pas toutes les implications de celui-ci.

En voulant supprimer une fonction drivenBy(Human/* const */ &) (voire, mieux encore, une fonction drivenBy(Driver/* const */ &) ) de ta notion de véhicule, tu ne fais pas que décider (volontairement) de ne pas respecter le DIP : tu décides volontairement "d'oublier" un fait de la plus haute importance : un véhicule ne peut être conduit que... s'il y a "quelqu'un" ou "quelque chose" pour le conduire.

(notes au passage que la version acceptant un Driver serait préférable du point de vue du DIP à la version acceptant un Human.  Il me semble qu'il existe déjà dés voitures conduite par... autre chose que des humains, ad minima à titre expériemental )

En décidant de supprimer cette fonction, tu décide aussi "d'amputer" ton véhicule du contexte dans lequel il est utilisé.

Or, tu as surement déjà croisé des interventions dans lesquelles on rappelait que toute donnée, tout type de donnée, toute fonction (tout module) ne vaut que par l'utilisation qui en est faite.

En d'autres termes : si tu écris une fonction à laquelle tu ne feras jamais appel, tu perds ton temps.  Mais le simple fait que l'on fasse appel à notre fonction en induit une autre : elle sera forcément appelée depuis une autre fonction.

Et cette fonction qui fait appel à la notre intervient forcément dans un contexte particulier.  Si tu retires ce contexte, la fonction appelante n'est plus jamais appelée, et notre fonction ne l'est par conséquent plus non plus.  Et elle ne sert donc désormais plus à rien.

Et puis, il y a des "choses" qui peuvent avoir leur "propre vie" (comme un conducteur, un animal ou que sais-je), même s'ils sont "pris en charge" par une intelligence artificielle, et il y en a d'autres (comme un véhicule ou un compte bancaire) qui ne sont là que... pour rendre un certain nombre de services bien particulier.

C'est, en sortant de l'approche purement orientée objet, exactement la différence que l'on fait entre un "être vivant" et un "objet".

Une perceuse, un tournevis ou un véhicule, ca rentre dans la catégorie des objets selon cette classification.  Cela veut dire qu'il n'auront de l'utilité que par et pour "l'être vivant" qui les manipule.

Une fois que tu auras compris cela, tu seras (peut-être) plus enclin à accepter l'idée de transmettre l'utilisateur d'un objet comme "contexte d'utilisation" de cet objet.  Et tu y seras sans doute d'autant plus enclin dans une approche OO -- à base de virtualité et de double dispatch --  au vu de tous les intérêts que peut apporter la connaissance du contexte d'utilisation.

Ton commentaire est beaucoup plus cadré, je trouve. Par contre ce qu'il ne semble pas préciser, c'est qu'on n'a pas besoin seulement d'encoder drivenBy : on a aussi besoin de drive.

Something::drivenBy(Driver &);
Driver::drive(Something &);

On a besoin d'encoder les deux et c'est là que se trouve la mocheté. D'un point de vue de la logique A conduit B est équivalent à B conduit par A. On ne devrait donc pas avoir à encoder les deux. Mais il se trouve qu'on n'a pas d'autres choix, du fait de la limite de nos langages de programmation.

misterFad a écrit:

Ton commentaire est beaucoup plus cadré, je trouve. Par contre ce qu'il ne semble pas préciser, c'est qu'on n'a pas besoin seulement d'encoder drivenBy : on a aussi besoin de drive.

Something::drivenBy(Driver &);
Driver::drive(Something &);

On a besoin d'encoder les deux et c'est là que se trouve la mocheté. D'un point de vue de la logique A conduit B est équivalent à B conduit par A. On ne devrait donc pas avoir à encoder les deux.

Et alors? où est le problème?

Comme tu l'as si bien fait remarquer, on ne conduit pas une moto comme on conduit une voiture (et c'est un motard dans l'âme, qui a néanmoins son permis voiture qui le dit :D).

Il est donc normal que l'on donne au conducteur le moyen d'adapter son comportement au véhicule qu'il conduit, tu ne trouves pas?

Tout comme il est normal que tu modifieras le comportement de conduite d'un véhicule particulier en fonction du "centre de décisions" (humain / VS intelligence artificielle) qui en prendra les commandes!

Et puis, encore une fois, tu es en C++ et non en java ou en C#.

Java et C# imposent une distinction artificielle entre la notion de classe et celle d'interface.  Et je comprend parfaitement leurs raisons de s'y prendre de la sorte.

Le truc, c'est que cette distinction artificielle pose un certain nombre de restrictions, dont la principale est que  toutes les fonctions exposées par une interface soient (au niveau de l'interface) "virtuelles pures" (pour reprendre un terme dont je ne sais pas s'il s'applique à d'autres langages que le C++)

C++ ne fait pas ce genre de distinction, et ne pose dés lors pas ce genre de restriction.

Si bien que si tu as une interface en C++ (He... ISP reste de stricte application) qui disposent des données suffisante pour permettre à une fonction de travailler correctement, il n'y a absolument aucune objection à que tu définisse un "comportement par défaut" pour cette fonction; et ce; que cette fonction soit virtuelle (et donc susceptible d'être redéfinie dans les classes dérivée) ou non.

On appliquera simplement d'autres techniques que celles de fonctions virtuelles pure pour s'assurer qu'une instance de l'interface ne pourra être créée qu'au travers des classes qui l'implémentent

Cette manière de travailler présente quantité d'avantages.  Dont (et tous les autres en découlent en fait... dont un meilleur respect de DRY)le fait que, tant qu'une classe implémentant cette interface peut se contenter du comportement par défaut défini dans l'interface, nous n'avons aucune raison de redéfinir le comportement en question

Mais il se trouve qu'on n'a pas d'autres choix, du fait de la limite de nos langages de programmation.

Voilà bien le noeud du problème

Dis toi bien que tu as encore de la chance d'être en C++ et non en java ou en C# sur ce coup là, car les restrictions qu'ils imposent sont encore bien plus strictes et finiraient sans doute par te rendre chèvre ...

Mais tout n'est qu'une question de patience : de nouveaux langages émergent de façon très régulière, et qui sait, peut être verrons nous de la sorte un langage apparaitre avec lequel il suffira d'indiquer (au niveau des classes de base, sinon, on en perd l'attrait) qu'une classe va travailler "main dans la main" avec un autre et que cette "autre classe" devra toujours manipuler le type le plus dérivé.

Bon, ce n'est surement pas pour demain; et je ne suis même pas sur que ce soit pour dans la décennie à venir... Mais qui sait?

-
Edité par koala01 22 septembre 2017 à 22:36:26

koala01 a écrit:

misterFad a écrit:

Ton commentaire est beaucoup plus cadré, je trouve. Par contre ce qu'il ne semble pas préciser, c'est qu'on n'a pas besoin seulement d'encoder drivenBy : on a aussi besoin de drive.

Something::drivenBy(Driver &);
Driver::drive(Something &);

On a besoin d'encoder les deux et c'est là que se trouve la mocheté. D'un point de vue de la logique A conduit B est équivalent à B conduit par A. On ne devrait donc pas avoir à encoder les deux.

Et alors? où est le problème?

Comme tu l'as si bien fait remarquer, on ne conduit pas une moto comme on conduit une voiture (et c'est un motard dans l'âme, qui a néanmoins son permis voiture qui le dit :D).

Il est donc normal que l'on donne au conducteur le moyen d'adapter son comportement au véhicule qu'il conduit, tu ne trouves pas?

Entièrement d'accord, mais cela devrait se faire d'un seul côté. En effet dès lors qu'on s'autorise à l'implémenter des deux côtés, forcément on se retrouve avec des fonctions (dans l'une ou l'autre des classes) qu'on ne peut pas s'autoriser à appeler, au risque de voir s'exécuter seulement une des surcharges de la fonction. Tu me diras qu'en C++ on peut rendre ces fonctions private/protected et utiliser la notion d'amitié (friend) mais c'est clairement du bricolage...

koala01 a écrit:

Mais il se trouve qu'on n'a pas d'autres choix, du fait de la limite de nos langages de programmation.

Voilà bien le noeud du problème

J'ai essayé de suivre la discussion, et il me semble que tu as omis quelque chose misterFad. Je me trompe sûrement ou tu l'as peut-être compris par la suite je ne sais pas.

misterFad a écrit:

C'est le principe d'avoir un visiteur abstrait, qui lui sait avec quels types il travaille, alors que les objets n'ont conscience que de ce visiteur abstrait (et pas de toutes les actions qui peuvent leur être appliquées !). Dans ton cas, non il n'y aurait pas besoin d'ajouter de nouvelles dépendances dans UndoCommand pour UndoStringifier, UndoFormatter, UndoPrinter, UndoStreamer. Dans l'idée simplement ça :

void UndoCommand::accept(const UndoDispatcher &dispatcher)
{
    dispatcher.dispatch(*this);
}

Par contre, cela pose le problème des retours de fonctions. Il faut que tous nos visiteurs concrets retournent le même résultat ?

misterFad a écrit:

Par exemple un appareil photo est capable de capturer un décor (objets, vivants, paysage, etc.) puis de l'encoder dans une image. Mais en aucun cas les éléments constitutifs du décor n'ont besoin de savoir ce qu'est un appareil photo, ni même d’être notifiés de son existence. De la même façon une commande n'a pas besoin de savoir ce qu'est une représentation textuelle du concept qu'elle définit, a fortiori si ladite représentation peut être déclinée sous plusieurs formes (et ce pour un même objet). Encore une fois, j'ai l'impression que opter pour le double dispatch n'est pas tant motivé par une convergence vers une solution fonctionnelle mais plutôt par l'envie d'éviter dynamic_cast. Et çà c’est triste.

De même, le décor n'a ici pas besoin de savoir ce qu'est un appareil photo (visiteur concret), juste de savoir qu'il peut être "visité" et que certains objets vont travailler avec. Cela nécessite donc d'y placer une méthode accept prenant en paramètre un pointeur/référence sur le visiteur abstrait, donc un peu intrusif je le concède, mais beaucoup moins que ce que tu suggères. Et surtout dans ce cas là, tu ne brises pas l'OCP puisque tu déclares ton objet "visitable", et de cette façon tout ajout d'action comme Formatter, Printer, Streamer peut se faire sans toucher à la classe UndoCommand !

Je ne sais pas si je réponds à ta question ou pas, mais je souhaitais éclaircir les points que j'ai cité.

-
Edité par Maluna34 25 septembre 2017 à 9:43:42

Maluna34 a écrit:

J'ai essayé de suivre la discussion, et il me semble que tu as omis quelque chose misterFad. Je me trompe sûrement ou tu l'as peut-être compris par la suite je ne sais pas.

misterFad a écrit:

• d'autre part on rajoute une dépendance supplémentaire dans UndoCommand : la classe dépend maintenant de UndoStringifier, ce qui n'a normalement pas lieu d'être. Cela suppose aussi qu'il faudrait refaire exactement la même chose si on venait à créer UndoFormatter, UndoPrinter, UndoStreamer, ... (certes c'est des noms de class un peu vague mais ça illustre mes propos).

C'est le principe d'avoir un visiteur abstrait, qui lui sait avec quels types il travaille, alors que les objets n'ont conscience que de ce visiteur abstrait (et pas de toutes les actions qui peuvent leur être appliquées !). Dans ton cas, non il n'y aurait pas besoin d'ajouter de nouvelles dépendances dans UndoCommand pour UndoStringifier, UndoFormatter, UndoPrinter, UndoStreamer. Dans l'idée simplement ça :

void UndoCommand::accept(const UndoDispatcher &dispatcher)
{
    dispatcher.dispatch(*this);
}

Ton commentaire m’amène à présumer que tu pars de l’hypothèse que UndoDispatcher est une classe de base pour UndoStringifier, UndoFormatter, etc. Pourtant on peut difficilement trouver un nom de fonction qui s’applique à tous les types de classes possible. Typiquement là, on se retrouve avec UndoStringifier::dispatch qui possède une valeur sémantique bien moindre que UndoStringifier::stringify. De même je pensais plutôt à CommandFormatter::format, ... Ces classes n’ont selon moi rien en commun (du moins elles s’occupent de tâches différentes), d’où ma remarque.

Maluna34 a écrit:

misterFad a écrit:

Par exemple un appareil photo est capable de capturer un décor (objets, vivants, paysage, etc.) puis de l'encoder dans une image. Mais en aucun cas les éléments constitutifs du décor n'ont besoin de savoir ce qu'est un appareil photo, ni même d’être notifiés de son existence. De la même façon une commande n'a pas besoin de savoir ce qu'est une représentation textuelle du concept qu'elle définit, a fortiori si ladite représentation peut être déclinée sous plusieurs formes (et ce pour un même objet). Encore une fois, j'ai l'impression que opter pour le double dispatch n'est pas tant motivé par une convergence vers une solution fonctionnelle mais plutôt par l'envie d'éviter dynamic_cast. Et çà c’est triste.

De même, le décor n'a ici pas besoin de savoir ce qu'est un appareil photo (visiteur concret), juste de savoir qu'il peut être "visité" et que certains objets vont travailler avec.

Nous partageons le même point de vue, du moins dans la première partie de ta phrase. Mais à vrai dire et comme je le mentionnais plus tôt, le décor ne devrait même pas avoir besoin de savoir qu’une quelconque entité le visite. Mais on en est arrivé là, tout simplement parce que nos langages de programmation ne permettent pas de faire autrement.

Maluna34 a écrit:

Cela nécessite donc d'y placer une méthode accept prenant en paramètre un pointeur/référence sur le visiteur abstrait, donc un peu intrusif je le concède, mais beaucoup moins que ce que tu suggères. Et surtout dans ce cas là, tu ne brises pas l'OCP puisque tu déclares ton objet "visitable", et de cette façon tout ajout d'action comme Formatter, Printer, Streamer peut se faire sans toucher à la classe UndoCommand !

Je ne sais pas si je réponds à ta question ou pas, mais je souhaitais éclaircir les points que j'ai cité.

Ce n’est pas tant au niveau de UndoCommand que l’on brise l’OCP. Le seul problème avec UndoCommand (ou la classe à visiter en général), c’est qu’on y rajoute une fonction dont elle n’a pas besoin d’un point de vue conceptuel. La fonction qu’on y rajoute ne luit sert pas : elle permet juste aux visiteurs de bien faire leur boulot. Pire encore, on se retrouve avec des visiteurs dont on ne peut même pas appeler les fonctions, au risque de voir s’exécuter seulement une des surcharges. Même ça, je le disais déjà dans mes précédents posts.

C’est plutôt au niveau des visiteurs que l’on brise l’OCP. En effet UndoStringifier n’a plus qu’une seule raison de changer. A chaque fois qu’une nouvelle commande est spécifiée (on hérite de Command), il faut automatiquement penser à (en tout cas il est souhaitable de) rajouter une nouvelle surcharge dans UndoStringifier afin de garantir le comportement attendu. UndoStringifier n’est par conséquent plus closed aux modifications. Cependant, comme je le disais plut tôt, une autre façon d’interpréter la situation serait de dire que c’est en fait la classe Stringifier qui était incomplète mais c’est clairement aller chercher l’argument juste pour se donner bonne conscience. Car, autant dire alors que l'on n'enfreint jamais l'OCP puisqu'à chaque fois il suffit de dire que c'est la classe qui n'était pas complète.

Mais ce n’est pas tout ! Il faut penser aussi à overrider la bonne fonction dans la nouvelle classe (héritant de Command) pour y mettre le code suivant

stringifier.stringifier(*this);

Sinon le résultat observé ne serait pas celui escompté. De même, dans le cas où l’on link avec une librairie dont on a n’a pas accès au code source (*.cpp), en créant une nouvelle classe héritant de Command, on se retrouve avec une fonction virtuelle (pure) que l’on peut (doit) overrider mais dont le paramètre (qui est un Stringifier) ne sera guère utile. On voit bien que les concepts du visiteur et du double dispatch ne sont pas exempts de défauts de conception, tout comme dynamic_cast qui, je vous l’accorde, implique fondamentalement un non respect du LSP. Dans les deux cas on est mal, mais on peut toujours s’autoriser à préférer l’une plutôt que l’autre.

Pour ma part, je vais finalement opter pour un bon vieux toString(). C’est moins souple mais au moins c’est beaucoup plus correct et plus Orienté Objet. Au passage, si le projet n’avait pas été une librarie, j’aurais pu me résoudre à utiliser le double dispatch (comme je le souhaitais au début), mais hélas.

Finalement j'ai reporté l'implémentation de la fonctionnalité, d'une part parce qu'elle n'est pas essentielle et d'autre part parce que les méthodes discutées jusque là sont peu satisfaisantes. toString() aurait pu fonctionner mais l'idéal aurait été de mettre en place un Stringifier. Cela implique néanmoins :

• l'utilisation de dynamic_cast (et donc le non respect du LSP)
• ou la simulation du pattern Visitor grâce au mécanisme du double dispatch (mais là aussi on enfreint un autre principe : le OCP)

Dans tous les cas, merci à tous pour vos réponses. Le projet en question est accessible ici. Je marque ce sujet en résolu.

Le dynamic_cast rompt aussi l'OCP. Et on n'a besoin que d'être capable de recevoir un visiteur abstrait dans l'AST pour le double dispatch.

C'est plein de sophiste votre réponse.

Il y a bien d'autre moyen d’implémenter des sérialisateur/désialisateur (dynamic_cast, et visitor n'en étant pas).

misterFad a écrit:

Ton commentaire m’amène à présumer que tu pars de l’hypothèse que UndoDispatcher est une classe de base pour UndoStringifier, UndoFormatter, etc. Pourtant on peut difficilement trouver un nom de fonction qui s’applique à tous les types de classes possible. Typiquement là, on se retrouve avec UndoStringifier::dispatch qui possède une valeur sémantique bien moindre que UndoStringifier::stringify. De même je pensais plutôt à CommandFormatter::format, ... Ces classes n’ont selon moi rien en commun (du moins elles s’occupent de tâches différentes), d’où ma remarque.

Mais ca, tu t'en fout pas mal! un dispatcher, ca dispatch. Point-barre.

Ca dispatche quoi? les commandes concrètes

Ca les dispatche comment? en fonction du type concret de dispatcher que tu mets en place

Si bien que n'importe quel dispatcher peut appeler n'importe quelle fonction membre exposée par le type concret de ta commande pour obtenir le résultat que l'on attend de sa part.

Il n'y a absolument pas à sortir de là

Ce n’est pas tant au niveau de UndoCommand que l’on brise l’OCP. Le seul problème avec UndoCommand (ou la classe à visiter en général), c’est qu’on y rajoute une fonction dont elle n’a pas besoin d’un point de vue conceptuel. La fonction qu’on y rajoute ne luit sert pas : elle permet juste aux visiteurs de bien faire leur boulot. Pire encore, on se retrouve avec des visiteurs dont on ne peut même pas appeler les fonctions, au risque de voir s’exécuter seulement une des surcharges. Même ça, je le disais déjà dans mes précédents posts.

C’est plutôt au niveau des visiteurs que l’on brise l’OCP. En effet UndoStringifier n’a plus qu’une seule raison de changer. A chaque fois qu’une nouvelle commande est spécifiée (on hérite de Command), il faut automatiquement penser à (en tout cas il est souhaitable de) rajouter une nouvelle surcharge dans UndoStringifier afin de garantir le comportement attendu. UndoStringifier n’est par conséquent plus closed aux modifications. Cependant, comme je le disais plut tôt, une autre façon d’interpréter la situation serait de dire que c’est en fait la classe Stringifier qui était incomplète mais c’est clairement aller chercher l’argument juste pour se donner bonne conscience. Car, autant dire alors que l'on n'enfreint jamais l'OCP puisqu'à chaque fois il suffit de dire que c'est la classe qui n'était pas complète.

Mais non, on ne brise pas l'OCP! bien au contraire!

L'OCP te dit que, une fois que tu as validé et testé le comportement le code d'une fonction, tu ne peux plus aller modifier ce code pour rajouter des fonctionnalités

Et c'est ce que le visiteur te permet de faire : tu ne vas pas modifier une classe existante pour rajouter des fonctionnalités, mais que tu vas créer une nouvelle classe pour implémenter ces nouvelles fonctionnalités, tu "n'as qu'à" rajouter une fonction qui accepte cette nouvelle classe et lui indiquerle comportement que tu souhaites obtenir.

Et tu n'as donc absolument aucun besoin d'aller modifier le code des fonctions qui existaient avant que tu ne décide de rajouter ces fonctionnalités

Mais ce n’est pas tout ! Il faut penser aussi à overrider la bonne fonction dans la nouvelle classe (héritant de Command) pour y mettre le code suivant

stringifier.stringifier(*this);

Ca, c'est quand même pas la mer à boire, non plus

Sinon le résultat observé ne serait pas celui escompté. De même, dans le cas où l’on link avec une librairie dont on a n’a pas accès au code source (*.cpp), en créant une nouvelle classe héritant de Command, on se retrouve avec une fonction virtuelle (pure) que l’on peut (doit) overrider mais dont le paramètre (qui est un Stringifier) ne sera guère utile. On voit bien que les concepts du visiteur et du double dispatch ne sont pas exempts de défauts de conception, tout comme dynamic_cast qui, je vous l’accorde, implique fondamentalement un non respect du LSP. Dans les deux cas on est mal, mais on peut toujours s’autoriser à préférer l’une plutôt que l’autre.

Sinon, le résultat obtenu devrait être une erreur de compilation à l'édition de liens, car la fonction que tu dois redéfinir est sensée être virtuelle pure

C'est déjà un peu tard (on aurait sans doute préféré qu'erreur survienne à la compilation au lieu de survenir à l'édition de liens), mais ce sera bloquant à un point tel que tu n'auras pas d'autre choix que de te souvenir qu'il faut définir cette fonction

misterFad a écrit:

De plus après avoir essayé les deux méthodes, il se trouve que enfreindre LSP est ce qu'il y a de plus "cohérent"/correct. Pourquoi ? Et bien tout simplement parce que s'il devrait y avoir un problème, on sait que ce sera forcément au niveau des Stringifier concrets, puisque c'est au niveau de ceux-ci que le dynamic_cast sera réalisé. A contrario dans le cas du double dispatch et comme je le disais dans mon précédent post, on se retrouve avec 2 dois plus de fonctions virtuelles, les unes étant impertinentes (conceptuellement parlant), les autres étant tout simplement inutilisables.

Dans tous les cas, merci à tous pour vos réponses. Le projet en question est accessible ici. Je marque ce sujet en résolu.

Ce n'est JAMAIS une bonne chose que de rompre le LSP.

A chaque fois que tu vas te dire "bas, ce n'est pas si grave dans le cas présent", tu peux te dire que tu t'en mordras les doigts plus tard.

Car il ne faut pas te leurrer: ton projet va évoluer au fil du temps.  Et la seule évolution possible ira dans le sens d'une complexité accrue.  De plus, tu dois t'attendre à oublier les raisons qui t'ont incité, à un instant T, à rompre le LSP.  Pire encore: aussi bonnes que puissent te sembler les raisons de rompre avec un principe de conception, tu dois t'attendre:

• à les oublier au fil du temps
• à ce qu'elles posent des pièges dans lesquels tu tomberas immanquablement plus tard
• à ce que, pour contourner les pièges posés par des décisions inopportunes, tu en viennes à rompre encore plus avec les principes de conception, créant des pièges encore plus fallacieux et beaucoup plus difficiles à éviter

koala01 a écrit:

misterFad a écrit:

Ton commentaire m’amène à présumer que tu pars de l’hypothèse que UndoDispatcher est une classe de base pour UndoStringifier, UndoFormatter, etc. Pourtant on peut difficilement trouver un nom de fonction qui s’applique à tous les types de classes possible. Typiquement là, on se retrouve avec UndoStringifier::dispatch qui possède une valeur sémantique bien moindre que UndoStringifier::stringify. De même je pensais plutôt à CommandFormatter::format, ... Ces classes n’ont selon moi rien en commun (du moins elles s’occupent de tâches différentes), d’où ma remarque.

Mais ca, tu t'en fout pas mal! un dispatcher, ca dispatch. Point-barre.

Ca dispatche quoi? les commandes concrètes

Ca les dispatche comment? en fonction du type concret de dispatcher que tu mets en place

Si bien que n'importe quel dispatcher peut appeler n'importe quelle fonction membre exposée par le type concret de ta commande pour obtenir le résultat que l'on attend de sa part.

C'est une façon de voir les choses mais celle-ci est à mon avis motivée par le fait de vouloir à tout prix déculpabiliser le double dispatch. En effet, CommandStringifier::dispatch n'a tout implement pas de sens. Encore une fois, la fonction que l'on rajoute dans les classes visitées est impertinente d'un point de vue conceptuel. Elle ne lui sert pas et permet juste aux visiteurs de pouvoir faire leur boulot. Et c'est l'une des choses qui me dérange depuis le début. Que tu me dises qu'on ne peut que s'en contenter, je comprendrais. Mais sinon on pourra difficilement s'entendre.

koala01 a écrit:

Mais non, on ne brise pas l'OCP! bien au contraire!

L'OCP te dit que, une fois que tu as validé et testé le comportement le code d'une fonction, tu ne peux plus aller modifier ce code pour rajouter des fonctionnalités

Au passage tu limites l'application de l'OCP seulement aux fonctions. Alors que non, celui-ci s'applique à la classe toute entière. Dès lors que le service rendu par une classe est entièrement implémenté/validé, on ne devrait plus modifier celle-ci : le service rendu est "scellé" mais on peut toujours l'étendre. A fortiori ici, on est obligé de rajouter une nouvelle surcharge à chaque fois qu'un nouveau type de commande est créé (une nouvelle classe hérite de Command).

koala01 a écrit:

Et c'est ce que le visiteur te permet de faire : tu ne vas pas modifier une classe existante pour rajouter des fonctionnalités, mais que tu vas créer une nouvelle classe pour implémenter ces nouvelles fonctionnalités, tu "n'as qu'à" rajouter une fonction qui accepte cette nouvelle classe et lui indiquerle comportement que tu souhaites obtenir.

Et tu n'as donc absolument aucun besoin d'aller modifier le code des fonctions qui existaient avant que tu ne décide de rajouter ces fonctionnalités

On en revient au CommandDispatcher et à CommandStringifier::dispatch. Tes remarques rejoignent en effet celles de Maluna34 et seraient légitimes si celles-ci ne reposaient pas sur l'enfreint d'une des règles de base de la POO à savoir : nommer proprement les différents éléments qui font du code source ce qu'il est. Je comprends que vous optiez pour Visitor/Double-Dispatch. Mais j'ai beaucoup du mal à comprendre en quoi ces concepts sont exempts de défauts.

koala01 a écrit:

Ce n'est JAMAIS une bonne chose que de rompre le LSP.

Entièrement d'accord, mais cette remarque est tout aussi valable pour OCP, ainsi que tous les autres principes.

koala01 a écrit:

Sinon le résultat observé ne serait pas celui escompté. De même, dans le cas où l’on link avec une librairie dont on a n’a pas accès au code source (*.cpp), en créant une nouvelle classe héritant de Command, on se retrouve avec une fonction virtuelle (pure) que l’on peut (doit) overrider mais dont le paramètre (qui est un Stringifier) ne sera guère utile. On voit bien que les concepts du visiteur et du double dispatch ne sont pas exempts de défauts de conception, tout comme dynamic_cast qui, je vous l’accorde, implique fondamentalement un non respect du LSP. Dans les deux cas on est mal, mais on peut toujours s’autoriser à préférer l’une plutôt que l’autre.

Sinon, le résultat obtenu devrait être une erreur de compilation à l'édition de liens, car la fonction que tu dois redéfinir est sensée être virtuelle pure

C'est déjà un peu tard (on aurait sans doute préféré qu'erreur survienne à la compilation au lieu de survenir à l'édition de liens), mais ce sera bloquant à un point tel que tu n'auras pas d'autre choix que de te souvenir qu'il faut définir cette fonction

Je pense que mon commentaire n'est pas suffisamment clair. En effet en partant de l'hypothèse que je n'utilise pas CommandDispatcher comme classe de base, ce que je dis est le suivant : cf. prochain paragraphe.

Lorsqu'un utilisateur récupère le code de la librairie mais qu'il n'a accès qu'aux headers et à une DLL, il se retrouve avec la fonction : 

Command::stringify(CommandStringifier &stringifier); // fonction au moins virtuelle
                                                     // éventuellement pure

Il est auorisé à overrider cette fonction. Mais pour autant le paramètre Stringifier ne lui servira à rien tout simplement par ce que pour que le stringifier fournisse le service attendu, il est nécessaire de rajouter une autre version overload de la fonction stringify. Et çà le développeur ne peut pas le faire puisqu'il n'a pas accès au code source. Et çà sans c'est compter le fait que toutes les fonctions publiques rajoutées dans les visiteurs ne servent à rien : on ne peut pas les appeler directement. Il faut toujours passer par leurs pendants rajoutées dans les classes visitées.

Le gros problème avec Visitor, c'est qu'il dépend fortement des implémentations concrètes. Même si on intégrait une classe de base comme CommandDispatcher, tout ce que cela garantirait, c'est qu'on ne serait plus obliger de refaire le même boulot pour d'éventuelles autres classes du genre CommandPrinter, CommandBlabla, ... Plus précisément, on ne serait pas obligé de modifier Command. Mais on aurait toujours besoin de rajouter (dans le visiteur, ici CommandStringifier) une nouvelle surchage à chaque fois qu'un nouveau stringifier est créé. Cela pourrait être pertinent dans certains cas mais dans le cas d'une librairie, c'est tout simplement pas terrible.

misterFad a écrit:

C'est une façon de voir les choses mais celle-ci est à mon avis motivée par le fait de vouloir à tout prix déculpabiliser le double dispatch. En effet, CommandStringifier::dispatch n'a tout implement pas de sens. Encore une fois, la fonction que l'on rajoute dans les classes visitées est impertinente d'un point de vue conceptuel. Elle ne lui sert pas et permet juste aux visiteurs de pouvoir faire leur boulot. Et c'est l'une des choses qui me dérange depuis le début. Que tu me dises qu'on ne peut que s'en contenter, je comprendrais. Mais sinon on pourra difficilement s'entendre.

Tapes la dans une interface, alors, et profites en pour respecter d'avantage l'ISP

De toutes façons, tu dois te dire que le double dispatch est -- à défaut d'être la meilleure solution -- la moins mauvaise des solutions que tu pourrais envisager lorsque tu as "été assez bête pour perdre le type concret d'une donnée"

Ca tu peux tourner les choses comme tu veux: toute autre solution travaillant à coup de (pseudo) RTTI ou de transtypage t'enverra systématiquement dans une contrée où tu éviteras un nid de scorpion pour tomber sur un nid de vipère et où tu passeras ton temps à débugger ton application pour arriver à comprendre pourquoi telle fonctionnalité n'est pas prise en compte dans telle situation

misterFad a écrit:

Au passage tu limites l'application de l'OCP seulement aux fonctions. Alors que non, celui-ci s'applique à la classe toute entière. Dès lors que le service rendu par une classe est entièrement implémenté/validé, on ne devrait plus modifier celle-ci : le service rendu est "scellé" mais on peut toujours l'étendre. A fortiori ici, on est obligé de rajouter une nouvelle surcharge à chaque fois qu'un nouveau type de commande est créé (une nouvelle classe hérite de Command).

Bien sur que je limite l'application de l'OCP au comportement des fonctions! Et c'est tout à fait normal, vu que les fonctions sont le seul endroit où il y a moyen de définir un comportement!

Tu dis toi-même que l'on peut étendre les fonctionnalités d'une classe. Mais il n'y a aucune différence entre se dire, à la longue "tiens, en fait, ce serait pas mal si ma classe pouvait répondre à la question "es-tu vide?" au lieu de devoir faire un if(truc.size()==0)" et se dire que "tiens, les circonstances ont évolué, et je dois rajouter une (surcharge de) fonction à ma classe pour qu'elle prenne un nouveau type de donnée en compte"

Il est vrai que, à choisir, la POO préférerait que l'on ait recours à l'agrégation ou à l'héritage, mais l'OCP n'a rien à voir là dedans: l'OCP s'intéresse aux comportements, c'est à dire au code qui permet d'indiquer aux différentes fonctions ce qu'elles doivent faire

misterFad a écrit:

On en revient au CommandDispatcher et à CommandStringifier::dispatch. Tes remarques rejoignent en effet celles de Maluna34 et seraient légitimes si celles-ci ne reposaient pas sur l'enfreint d'une des règles de base de la POO à savoir : nommer proprement les différents éléments qui font du code source ce qu'il est. Je comprends que vous optiez pour Visitor/Double-Dispatch. Mais j'ai beaucoup du mal à comprendre en quoi ces concepts sont exempts de défauts.

Tu n'y vois des défauts que parce que tu butes sur le type concret de ton visiteur; sur le fait que, si tu as un visiteur dont le but est de <faire une chose quelconque> il est "normal" de pouvoir appeler ce comportement par son nom.

(Ce en quoi tu n'as d'ailleurs pas tout à fait tord)

Mais tu oublie un principe dans l'histoire: le DIP, qui te dit que tes modules doivent dépendre d'abstractions, ce qui te met -- de facto -- dans une situation dans laquelle... tu es assez bête que pour oublier le type réel de l'élément que tu utilises.

Si bien que, quel que soit l'objectif réel de ton visiteur ou de ton dispatcher concret, tu l'auras perdu de vue (bien que le visiteur ou le dispatcher le sache pertinemment), et que donc, tu dois "tout simplement" en revenir aux basiques : un visiteur visite, et un dispatcher ... dispatche.

D'ailleurs, tu pourrais tout aussi bien avoir un code qui ressemble à

int main(){
   using dispatchPtr_t = std::unique_ptr<Dispater>;
   using visiteePtr_t = std::unique_ptr<Visitable>;
   std::vector<dispatchPtr_t> allDispatchers;
   std::vector<visiteePtr_t> allItems:
   /* on remplit les tableaus avec différentss dispatchers
    * / éléments visitables concrets
    * je laisse cela en suspend 
    */
   Dispatcher * selected{nullptr};
   if(condition1)
       selected = allDispatchers[0].get();
   else if(condition2)
       selected = allDispatchers[1].get();
   else if(condition3)
       selected = allDispatchers[2].get();
   else if(condition4)
       selected = allDispatchers[3].get();
   if(selected){
       for(auto & temp : allItems){
           temp.get()->accept(*selected));
           /* avec temp->accept(Dispater & d){
            *    d.dispatch(*this);
            * }
            */
       }
   }
   /* ... */
}

Avec un tel code, tu te rends bien compte que tu ne sais même pas exactement quel est le type de ton dispatcher (même si l'OCP n'est pas respecté à cause des if ... else en pagaille) et que ton élément ne doit savoir qu'une seule chose : comment s'adresser à... un Dispatcher

misterFad a écrit:

Entièrement d'accord, mais cette remarque est tout aussi valable pour OCP, ainsi que tous les autres principes.

Cette remarque s'applique à tout principe et à toute loi de conception

On parle ici de tous les principes SOLID, mais on pourrait dire la même chose pour la loi de Déméter ou pour tous les principes relativement proches des principes SOLID que l'on pourrait citer

misterFad a écrit:

Je pense que mon commentaire n'est pas suffisamment clair. En effet en partant de l'hypothèse que je n'utilise pas CommandDispatcher comme classe de base, ce que je dis est le suivant : cf. prochain paragraphe.

Lorsqu'un utilisateur récupère le code de la librairie mais qu'il n'a accès qu'aux headers et à une DLL, il se retrouve avec la fonction : 

Command::stringify(CommandStringifier &stringifier); // fonction au moins virtuelle
                                                     // éventuellement pure

Il est auorisé à overrider cette fonction. Mais pour autant le paramètre Stringifier ne lui servira à rien tout simplement par ce que pour que le stringifier fournisse le service attendu, il est nécessaire de rajouter une autre version overload de la fonction stringify. Et çà le développeur ne peut pas le faire puisqu'il n'a pas accès au code source. Et çà sans c'est compter le fait que toutes les fonctions publiques rajoutées dans les visiteurs ne servent à rien : on ne peut pas les appeler directement. Il faut toujours passer par leurs pendants rajoutées dans les classes visitées.

Le gros problème avec Visitor, c'est qu'il dépend fortement des implémentations concrètes. Même si on intégrait une classe de base comme CommandDispatcher, tout ce que cela garantirait, c'est qu'on ne serait plus obliger de refaire le même boulot pour d'éventuelles autres classes du genre CommandPrinter, CommandBlabla, ... Plus précisément, on ne serait pas obligé de modifier Command. Mais on aurait toujours besoin de rajouter (dans le visiteur, ici CommandStringifier) une nouvelle surchage à chaque fois qu'un nouveau stringifier est créé. Cela pourrait être pertinent dans certains cas mais dans le cas d'une librairie, c'est tout simplement pas terrible.

Ah, ok, je vois ce que tu veux dire. Et je te rassure, il y a moyen de contourner le problème à l'aide de l'idiome NVI ou de l'idiome dit "pointer implementation" (pimpl idiom), voire à l'aide des deux

Mais, avant de te projeter dans les affres de la distribution de bibliothèques dont tu ne fournis pas le code source, commence déjà par t'habituer à respecter les principes fondamentaux à la lettre.  Une fois que tu seras suffisamment à l'aise avec leur mise en place, tu verras, le reste viendra pour ainsi dire naturellement