Bonjours à tous,

Un peu de contexte, un peu de code :

struct Data {
	std::string name;
	int value;
	size_t id;
};

class Model {
public:
	Model() :nextID{ 0 }
	{}

	void addData(std::string name, int value) {
		data.push_back(Data{ name, value, nextID });
		nextID++
	}

private:
	std::vector<Data> data;
	size_t nextID;
};

On m'a dit sur un précédent sujet que pour respecter la loi Demeter, l'utilisateur ne doit pas avoir connaitre la structure Data pour utiliser Model , d'où la fonction membre addData

Mon problème provient du fais que ces Data sont sauvegarder dans un fichier (et leur ID aussi), et sont lu par une fonction qui renvoie directement des Data :

Data readData(){
   ...
   return data;
}

Du coup, quel code faut-il préférer si on souhaite ajouter ces données au Model :

void loadData(Model & model) {
	model.addData(readData());
}

Ou

void loadData(Model & model) {
	Data data{ readData() };

	model.addData(data.name, data.value, data.id);
}

(En surchargeant la fonction membre addData comme il se doit, bien entendu)

Pour moi, le premier code est plus propre/concis, mais l'utilisateur peut (mais ne doit pas) connaitre la structure Data  pour utiliser Model, et je n'arrive pas à savoir si cela respecte la loi Demeter (malgré plusieurs relecture de sa définition).

Je doute qu'il puisse y avoir des remarque H.S sur ce simple exemple, mais si jamais, je suis preneur

Merci d'avance pour votre lecture/aide

Il y a une petite subtilité, je vais prendre un contexte un peu différent. Lorsque tu fais une requête sur une base de données, tu n'as pas d'autre choix que de connaître la structure de la base de données, sans la description de la table, impossible d'écrire la requête. En revanche comment le SGBD stocke ses données est sans importance. C'est là que se trouve la subtilité de la loi de Demeter, tu récupères tes données sans avoir besoin de connaître les détails du SGBD, mais pour ce qui est des données, ce sont les données, c'est la spécification...

Lu'!

Data est un agrégat de données, pas un objet. Donc à partir de là, l'importance devient moyen cruciale.

int21h a écrit:

Il y a une petite subtilité, je vais prendre un contexte un peu différent. Lorsque tu fais une requête sur une base de données, tu n'as pas d'autre choix que de connaître la structure de la base de données, sans la description de la table, impossible d'écrire la requête. En revanche comment le SGBD stocke ses données est sans importance. C'est là que se trouve la subtilité de la loi de Demeter, tu récupères tes données sans avoir besoin de connaître les détails du SGBD, mais pour ce qui est des données, ce sont les données, c'est la spécification...

La subtilité disparait un peu avec les langages qui ont une notion d'interface. On sait qu'une fonction retourne une instance d'une classe qui implémente une certaine interface qui spécifie ce qu'elle doit savoir faire, tant que le code de la classe respecte l'interface, y a pas de problème si on change le code de la classe.

Un point important, c'est que la loi de Déméter, c'est pas un principe d'inspiration divine gravé dans le marbre sous peine d'être frappé par la foudre, c'est une manière de formuler qu'on veut éviter d'avoir du code dans une classe A qui dépende de détails du code d'une autre classe B. Parce ce que si on doit faire évoluer B, il va falloir retoucher le code de A, ça va être emmerdant.

C'est donc un principe qui part d'un bon sentiment, mais qu'il faut, au mimimum, interpréter, au lieu de l'appliquer à toutes les sauces avec le zèle fanatique de ceux qui veulent avoir des lois pour ne pas avoir à réfléchir.

• d'une part (comme le dit @Ksass-Peuk), quand une classe/structure sert juste à représenter un "Plain Old Data" object, un simple regroupement de champs. Le POD, il est pas là pour introduire une abstraction, il est là pour faire un paquet pour transporter plusieurs informations d'un coup.  On va pas s'emmerder à cacher les champs, pour y coller des accesseurs, et ensuite être emmerdés avec les accesseurs qu'on n'aurait pas le droit d'utiliser. Dans ce cas, aucun gain, que des complications, donc code moins maintenable : résultat contraire aux objectifs de toutes les lois du génie logiciel.
• un autre cas, c'est les API fluides (fluent interface), où on écrit délibérément des choses comme ça (style java)

personnel.stream()
         .filter( agedMore(65) )
         .map(Employe::getName)
         .foreach(sayGoodBye);

Délibérément, et avec bonheur : stream() retourne un flot d'employés, filter en produit un autre, map produit un flot de noms, et foreach applique sayGoodBye à chaque nom.

Le pipeline à 5 niveau pique les yeux au début, mais en fait ne contourne pas la loi de Déméter  https://www.markwithall.com/programming/2015/05/03/fluent-apis-and-the-law-of-demeter.html  : on n'accès pas au détail des classes qui sont mise en oeuvre, on ne fait qu'appliquer le fait que l'interface Stream (implémentée par les objets retournés par stream()) a des méthodes fiilter, map et foreach.

C'est pas pire que

std::cout << "Hello, "
          << name
          << "!"
          << std::endl;

qu'on se permet d'écrire parce qu'on sait très bien que << retourne un ostream.

PS si tu n'utilise pas ta struct data en dehors de la classe Model, dans un langage bien conçu, elle serait déclarée DANS la classe.

Alternative: elle peut être déclarée comme type dans Model.cpp, sans la faire apparaitre dans ce que le "module" Model exporte (via le Modele.h) pour être utilisé par d'autres classes.  C'est toujours une notion d'interface.

-
Edité par michelbillaud 30 août 2019 à 16:36:11

Salut,

De manière générale, tu dois te poser plusieurs questions au sujet de l'agrégat que tu souhaites utiliser.

La première est sans doute "l'utilisateur de l'agrégat a-t-il besoin d'en connaitre systématiquement l'ensemble des champs qui le composent en même temps?"

Dans l'exemple que tu présente, on peut être à peu près sur que l'utilisateur voudra régulièrement accéder au champs value très rapidement avant (ou après) avoir accéder au champs name, mais les choses sont beaucoup moins claires en ce qui concerne le champs id.

Car, il n'y a rien à faire : si le champs name et le champs value sont définis de manière externe à ton modèle, c'est qu'il y a une bonne raison à cela.  Par contre, si le champs id est défini par ton modèle,c'est qu'il y a aussi une bonne raison

Sans rien savoir de ton projet, on peut donc partir sur deux possibilités différentes concernant le champs id:

• soit c'est un champs qui ne sert que pour ton modèle n'a aucun besoin d'y accéder,
• soit ce champs peut être "diffusé" pour permettre au reste de travailler, mais l'usage doit en être restreint.  La deuxième question qu'il faut se poser te permettra de mieux comprendre ce que je veux dire

La deuxième question à se poser est "quelles catastrophes l'utilisateur peut-il occasionner s'il se met à modifier impunément les différents champs de ton agrégat?"

Le champs name, par exemple, risque déjà de poser pas mal de problème si l'utilisateur est en mesure de le modifier, parce que "nommer, c'est créer" : si il change le nom de l'instance de l'agrégat, il risque de changer le sens général donné à l'instance.

Mais les choses sont encore pires au niveau du champs id, car, si cette valeur est déterminée par ton modèle, c'est -- très clairement -- parce qu'elle a une signification particulière pour le modèle. Si l'utilisateur peut modifier impunément cette valeur, il y a neuf chances sur dix (pour ne pas dire d'avantage) que la "nouvelle valeur" qu'il donnera à une instance particulière rentre en conflit avec la valeur "légitime" d'une autre.

Bien sur, cela foutra à tous les coups  le bordel au niveau du modèle, et qui pourrait l'étendre à tous les éléments qui, pour une raison ou une autre, utilise ce champs id pour ce qu'il est sensé représenter : un champs qui permet d'identifier sans l'ombre d'un doute chaque instance de ton agrégat de manière bien spécifique.

La troisième question qui viendra "naturellement à l'esprit" sera sans doute "comment éviter tous ces problèmes?".  La réponse pouvant allègrement varier (en fonction de la réponse à la première question) entre "en modifiant l'agrégat pour rendre certains champs immuables", par exemple en lui donnant la forme de

struct Data{
    std::string const name;
    int value;
    size_t const id;
};

et "en sortant les champs inutiles de l'agrégat", par exemple en créant un agrégat "InternalData" au niveau du modèle sous une forme proche de

struct Data{
    std::string const name;
    int value;
    /* le champs "id est destiné à l'usage interne du 
     * modèle, il n'a donc rien à faire ici
     */
};
class Model{
    struct InternalData{
        Data d;
        size_t id;
    };
public:
    void add(std::string const & name, int value){
        datas.push_back(InternalData{Data{name,value},nextId});
    ++nextId;
    }
    /* on peut désormais renvoyer une donnée de type
     * Data, qui ne contient que les champs utiles
     * et accessibles en renvoyant datas[xx].d;
     */
private:
    std::vector<InternalData> datas;
    size_t nextId;
};

Enfin, la dernière question à se poser -- et sans doute la plus importante -- est sans doute "quel est le role joué par le modèle?"

Si le modèle doit agir comme une "simple collection de données", il semble "logique" qu'il finisse par donner accès à "un certain nombre" de fonctionnalités issues de la classe std::vector, par exemple, en prenant la forme de

class Model{
public:
    using iterator = typename std::vector<Data>::iterator;
    using const_iterator = typename std::vector<Data>::const_iterator;
    void add(std::string const & name, int value){
       datas.push_back(Data{name, value, nextId});
       ++nextId;       
    }
    size_t size() const{
        return datas.size();
    }
    iterator begin(){
        return datas.begin();
    }
    iterator end(){
        return datas.end();
    }
    const_iterator begin() const{
        return datas.begin();
    }
    const_iterator end() const{
        return datas.end();
    }
    Data & operator[](size_t index){
        assert(index < datas.size() && "index out of bound");
        return datas[index];
    }
    Data const & operator[](size_t index) const{
        assert(index < datas.size() && "index out of bound");
        return datas[index];
    }
private:
    std::vector<Data> datas;
    size_t nextId{1};
};

car sa responsabilité se limite à ... maintenir les données en mémoire et y donner accès.  Dans ce cas, Déméter n'a -- effectivement pas grand chose à dire, parce que tout les traitements que l'on pourrait envisager au niveau de l'agrégat prennent place ... en dehors du modèle 

Si son rôle est de s'assurer que les modifications apportées à l'agrégat restent malgré tout cohérentes, une chose est sure : il devra tout faire pour que l'utilisateur ne puisse en aucun cas modifier une instance de l'agrégat par lui-même et, dans le pire des cas, ne permettre d'obtenir que des const_iterator ou une référence constante à l'agrégat.

Et, dans ce cas, Demeter serait parfaitement respecté, vu que l'utilisateur n'aurait effectivement aucun besoin de connaitre le type Data pour pouvoir manipuler à son aise le modèle. 

Reste, bien sur, le cas où l'utilisateur accèderait à une version constante de l'agrégat, qui impliquerait qu'il doive connaitre la structure Data pour pouvoir l'interroger sur les différentes valeurs de ses champs, mais on sort alors du cadre  d'une "manipulation du modèle"