Discussion :

[Agoudard]

Bonsoir,

Je suis régulièrement confronté à un choix et je n'arrive pas à me décider.

Si une objet A comporte un std::vector, et qu'un autre objet B cherche à accéder à tous les éléments de ce vector :
- Il vaut mieux écrire les accesseurs de A utilisés par B qui permettent cette opération. Fastidieux mais opaque.

Où

- Il vaut mieux retourner std::vector<MonType>&. Simple mais transparent.

[koala01]

Salut,

Commence d'abord par te poser la question de savoir s'il est cohérent que ton objet de type C puisse accéder aux éléments (de type B) contenus dans le vecteur du type A...

En effet, la loi demeter nous incite à veiller à ce que, si un objet de type A contient un objet de type B, un objet de type C connaissant l'objet de type A ne devrait pas pouvoir accéder, au départ de l'objet de type A, à l'objet de type B, dont C ne devrait d'ailleurs avoir aucune connaissance.

Le corolaire de cette loi est que la réponse dépendra principalement de la responsabilité que tu envisage de donner pour A.

Dans certains cas, il sera plutot opportun de fournir des fonctions membres à A qui permettent "simplement" d'effectuer certains traitement sur tout ou partie du contenu de la collection de B sans pour autant exposer le fait que A manipule des B, les B n'étant alors considéré que comme... des données permettant à A de fournir les services que l'on attend de sa part

Dans d'autres, la classe A se "substitue" à la collection (au vector) et sa responsabilité est alors de gérer les objets de type B.

L'approche la plus maléable est alors de se dire que le vector n'est en réalité qu'un détail d'implémentation, qui pourrait parfaitement être remplacé par n'importe quel autre type de conteneur présentant une interface identique (list, set ou autre), mais que l'utilisateur de A n'a strictement aucun besoin de savoir si les différents objets sont bel et bien placés dans un vector, ou s'ils sont en réalité placés dans une list

Dans ce cas là, quelques typedef de visibilité bien choisie et quelques fonctions utiles représentent, à mon sens, la meilleur alternative.

Il peut, en effet, paraitre fastidieux de créer les typedefs et les fonctions membres, mais le temps "perdu" à le faire sera très largement récupéré en terme de facilité et de sécurisation d'utilisation, voire, en terme de décision différente ultérieure.

Ainsi, il faut comprendre que si tu renvoie une référence sur le conteneur (quel qu'il soit) non constante au départ d'une fonction membre non constante de ta classe, tu ouvre la porte à une modification incontrôlée et incontrôlable du contenu de celui-ci, sans compter le fait que tu rend bien plus compliquée la décision éventuelle de changer le type de la collection parce que certains bench en auraient démontré l'opportunité

En un mot, et bien que cela puisse paraitre paradoxal, je dirais presque que ton but doit être de te compliquer la vie afin de faciliter celle de ceux qui utiliseront ton travail

Pour moi, la meilleure manière de t'y prendre, s'il est acquis que A se substitue réellement à une collection, serait donc de partir d'une classe A ressemblant à quelque chose comme

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
class A
{
    /* ce typedef est privé par défaut, car à usage interne uniquement */
    typedef std::vector<UnType> vector;
    public:
        /* il faudra surement l'itérateur constant */
        typedef vector::const_iterator const_iterator;
        /* et  * peut être * l'itérateur non constant */
        typedef vector::iterator iterator;
        /* on rajoute les fonctions qui vont bien */
        A(){}
        template <typename iterator>
        A(iterator b, iterator e):tab(b,e){}
        const_iterator begin() const{return tab.begin();}
        const_iterator end() const{return tab.end();}
        const_iterator find(/* paramètres */) const; // si elle est utile
        size_t size() const{return tab.size();}
        void add(UnType);
        /* toutes les autres fonctions utiles (éventuellement non constantes) 
         */
    private:
        vector tab;
}

[Agoudard]

Merci

J'ai rien compris à la loi Demeter.
Ça signifie qu'une méthode d'un objet A ne doit pas accéder à une collection d'objet dans un autre objet B qui se trouve dans A ?
EDIT : Google est mon amis, Oui la loi demeter c'est ça...

Mais comme tu dis, masquer le fait qu'il s'agit d'un std::vector est une chose importante.

[Emmanuel Deloget]

Mais comme tu dis, masquer le fait qu'il s'agit d'un std::vector est une chose importante.

C'est même certainement le point le plus important, car ainsi tu masque ton implémentation, forçant l'utilisateur à utiliser ton interface (paradigme program to interface). Tu t'assure ainsi d'un meilleur contrôle de l'invariant de la classe, puisque l'état de ta classe ne peut être modifié sans passer par le chemin que tu imposes.

Bien évidemment, ça nécessite de prévoir une interface bien pensée, ce qui te force à réfléchir aux objectifs que tu t'es fixé et à la place de cette classe dans ton plan global.

Bref, bien programmer, ça force à bien concevoir

[CedricMocquillon]

Je reste dubitatif (je suis comme le PO, je me pose cette question depuis un moment déjà).

@koala01 : si la classe A dispose en interne d'un autre vecteur, l'interface peut vite devenir compliquée

Personnellement, j'en suis arrivé à retourner une référence constante sur le vecteur interne avec une méthode add. J'utilise énormément du concept de range, donc j'ai pas vraiment besoin des begin / end

Mon design m'amène souvent dans ce cas à considérer que A doit gérer l'aspect collection (je restreint donc la possibilité d'ajouter des éléments) par contre A ne doit pas contrôler le contenu (mes vecteurs sont alors des vecteurs de pointeurs ce qui permet lors du renvoi de la référence constante sur le vecteur d'interdire l'ajout / suppression d'éléments mais autorise l'appel de méthodes non constantes sur mes objets)

ex:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
 
#define foreach BOOST_FOREACH
 
struct A
{
  typedef std::vector<std::shared_ptr<MonType>> lstTypes;
 
  void add(std::shared_ptr<MonType>& elt);
  //il m'arrive aussi d'utiliser des méthodes add avec les paramètres nécessaires à la construction d'un MonType
 
  lstTypes const& mes_types() const;
 
private:
   lstTypes mes_types_m;
};
 
int main()
{
  A a;
  using namespace boost::adaptors;
  foreach(UnType& t, a.mes_types() | indirected)
  {
    //faire un truc avec t mais pas forcement de méthodes constantes
  }
}

[koala01]

Merci

J'ai rien compris à la loi Demeter.
Ça signifie qu'une méthode d'un objet A ne doit pas accéder à une collection d'objet dans un autre objet B qui se trouve dans A ?
EDIT : Google est mon amis, Oui la loi demeter c'est ça...

Un petit exemple, ca vaudra sans doute mieux qu'un grand discours :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
class B
{
    public:
        void doSomthing()/* const */ {/*...*/}
};
class A
{
    private /* ou protected */:
    B b;
};
class C
{
    public:
        void foo(A /* const & */ a);
};

Si tu veux invoquer la fonction doSomething du membre b de type B qui se trouve dans la classe A, tu as la mauvaise méthode qui consiste à offrir un accesseur sur b:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
class A
{
   public:
       B /* const & */ getB() /* const{return b;}
       /* comme plus haut */
};
void C::foo(A /* const & a)
{
    a.getB().doSomething();
}

En effet, bien que *relativement simple*, cette solution oblige le type C à connaitre le type B, alors qu'il doit déjà connaitre le type A.


Et tu as la bonne méthode qui consiste à se dire que la classe A connait déjà la classe B et sait donc qu'il existe la fonction B::doSomething.

Il n'y a donc aucune raison à forcer C à connaitre la classe B: Par contre, on peut estimer que l'invocation de b::doSomething fait partie des services rendus (éventuellement à l'intérieur d'une logique plus complexe) par la classe A.

De cette manière, celui qui utilise la classe A (C dans mon exemple) ne doit connaitre que la classe A, et rien d'autre :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
class A
{
    public:
        void anyService() /* const */
        {
            /* éventuellement quelque chose avant */
            b.doSomething();
        }
    private:
        B b;
};
void C::foo(A /* const & */ a)
{
    a.anyService();
}

Je reste dubitatif (je suis comme le PO, je me pose cette question depuis un moment déjà).

@koala01 : si la classe A dispose en interne d'un autre vecteur, l'interface peut vite devenir compliquée

Elle ne devrait pas...

Il y a en effet un autre principe qu'il s'agit de garder en tête: il est connu sous le terme de SRP (Single Responability Principle, ou principe de la responsabilité unique, si tu préfère en francais )

C'est ce fameux principe qui dit, entre autre, que si une fonction fait plus d'une chose, c'est sans doute qu'elle en fait trop.

Mais ce n'est pas seulement vrai pour les fonctions, ce l'est aussi pour les classes et les structures

Même en généralisant au mieux la responsabilité que l'on souhaite donner à une classe qui manipule une collection d'objets, on reste dans l'ordre de "sa resposabilité est de permettre la gestion des différents objets".

Mais, comme Emmanuel l'a fait remarquer, l'idéal reste toujours de veiller à ne

pas exposer le type de collection qui est manipulé par la classe
Personnellement, j'en suis arrivé à retourner une référence constante sur le vecteur interne avec une méthode add.

C'est un tord

J'utilise énormément du concept de range, donc j'ai pas vraiment besoin des begin / end

Et pourtant, si tu présente simplement des fonctions qui renvoient un itérateur:

1. begin permet d'obtenir le premier élément sans se poser de question
2. end permet de s'assurer que l'itérateur renvoyé est valide
3. il manque "juste" une fonction find qui te permette de déterminer les deux extrêmes de ton range (outre, bien sur, les fonctions permettant de rajouter des éléments

Mon design m'amène souvent dans ce cas à considérer que A doit gérer l'aspect collection (je restreint donc la possibilité d'ajouter des éléments) par contre A ne doit pas contrôler le contenu (mes vecteurs sont alors des vecteurs de pointeurs ce qui permet lors du renvoi de la référence constante sur le vecteur d'interdire l'ajout / suppression d'éléments mais autorise l'appel de méthodes non constantes sur mes objets)

Tu peux parfaitement renvoyer des itérateurs non constants, s'il apparait que la modification de l'élément n'occasionne pas la modification de l'objet gérant la collection... mais ce n'est pas forcément le cas

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
 
#define foreach BOOST_FOREACH
 
struct A
{
  typedef std::vector<std::shared_ptr<MonType>> lstTypes;
 
  void add(std::shared_ptr<MonType>& elt);
  //il m'arrive aussi d'utiliser des méthodes add avec les paramètres nécessaires à la construction d'un MonType
 
  lstTypes const& mes_types() const;
 
private:
   lstTypes mes_types_m;
};
 
int main()
{
  A a;
  using namespace boost::adaptors;
  foreach(UnType& t, a.mes_types() | indirected)
  {
    //faire un truc avec t mais pas forcement de méthodes constantes
  }
}

Là, tu n'utilise pas vraiment un range: tu invoque de manière systématique une série de fonctions sur l'ensemble du contenu

Par contre, avec begin et end, tu peux *réellement* travailler sur des ranges, et ce sera encore plus le cas avec la nouvelle norme qui autorise

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

std::for_each(iter_begin, iter_end, functor);

[CedricMocquillon]

Je suis d'accord avec ta position et celle d'Emmanuel, je me fait ici un peu l'avocat du diable :-)

J'aurai du dans mon exemple remplacer le renvoi d'une référence constante sur le vecteur par un sub_range de boost qui correspond plus à mon intention initiale : renvoyer une "paire d'itérateurs" ayant l'interface d'une collection.

Ce que je voulais illustrer c'est que pour moi le code "client" de la classe A ne doit pas faire (trop) d'hypothèses sur la représentation interne des objets de A (notamment au niveau de la liste (ici pris au sens large du terme) des MonType). Utiliser un BOOST_FOREACH ou encore un boost::for_each (de boost/range/algorithm) ou encore BOOST_AUTO ou encore auto (de C++0x) permet de "récupérer" lstTypes et de travailler dessus sans "trop" faire d'hypothèses sur le type de collection utilisé en interne.

Je te concède que c'est également le cas avec les itérateurs pour autant, on a toujours des limites au fait de cacher la représentation interne, avec un itérateur (ou un sub_range): si on change la représentation interne de la collection des MonType d'un vector à une map, le code client sera impacté (on ne peut plus simplement déréférencer l'itérateur pour accéder à l'élément, on est obligé de faire un truc style it->second).

On peut encore dans ce cas, changer le type des itérateurs exposés (je pense notamment à iterator transform) pour masquer ce changement, il n'empêche que modifier le mode de gestion des objets (passer d'un vector<MonType> à un vector<shared_ptr<MonType>>) n'est pas non plus trivial à "masquer" pour le code client.

Ça me rappelle un bouquin (de Meyer il me semble, genre Effective STL) qui indiquait qu'il y a des limites à l'interchangeabilité des types de conteneurs et que ce n'est pas non plus ce qu'il fallait forcément chercher dans nos codes.

Au final, je suis d'accord avec le SRP et Demeter c'est juste que c'est parfois un brin "plus simple" de faire une petite entorse à la règle notamment quand on a un contrôle total sur le code "client". Après je ne tiendrais pas le même discours pour le développement d'une bibliothèque "générique" à plus large diffusion.

Enfin dernier point, malgré le SRP, il m'arrive d'avoir à gérer au sein d'une même classe, plusieurs collection (c'est plutôt l'exception mais ça m'arrive). Prenons l'exemple d'une ferme avec ses animaux, j'aurais tendance à écrire un code proche de:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
 
class ferme
{
public:
 
  void add(volaille);
  void add(canard);
  void add(poulet);
  void add(cochon);
 
  lstAnimaux animaux() const;
  lstVolailles volailles() const;
  lstCanards canards() const;
  lstPoulets poulets() const;
  lstCochons cochons() const;
 
private:
  std::vector<animal*> animaux_m;
  std::vector<volaille*> volailles_m;
  std::vector<canard*> canards_m;
  std::vector<poulet*> poulets_m;
  std::vector<cochon*> cochons_m;
};

Avec volaille et cochon héritant d'animal et canard et poulet héritant de volaille.

Personnellement, je ne considère pas (je fait peut être une erreur ici) que Demeter est "violé", un code client qui compte manipuler une ferme sans connaître quels sont les animaux présents ne me paraît pas réaliste.

[Emmanuel Deloget]

Je reste dubitatif (je suis comme le PO, je me pose cette question depuis un moment déjà).

@koala01 : si la classe A dispose en interne d'un autre vecteur, l'interface peut vite devenir compliquée

Si la classe en interne possède un autre vecteur, alors son rôle n'est clairement pas celui d'un conteneur - du coup, son interface se doit de le refléter. Elle ne devient donc pas nécessairement plus compliquée.

Personnellement, j'en suis arrivé à retourner une référence constante sur le vecteur interne avec une méthode add. J'utilise énormément du concept de range, donc j'ai pas vraiment besoin des begin / end

Mon design m'amène souvent dans ce cas à considérer que A doit gérer l'aspect collection (je restreint donc la possibilité d'ajouter des éléments) par contre A ne doit pas contrôler le contenu (mes vecteurs sont alors des vecteurs de pointeurs ce qui permet lors du renvoi de la référence constante sur le vecteur d'interdire l'ajout / suppression d'éléments mais autorise l'appel de méthodes non constantes sur mes objets)

L'intérêt d'encapsuler un vecteur, c'est de permettre d'effectuer des traitements plus spécialisés sur ce vecteur. Dans on exemple, il eut été plus simple de proposer un méthode foreach() dans ta classe (prenant par exemple un foncteur en paramètre) plutot que d'exposer la collection interne.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
 
struct A // la même que ta classe, plus : 
{
public:
  template <class Callable> void foreach(Callable callable)
  {
    std::for_each(mes_types_m.begin(), mes_types_m.end(), callable);
  }
  typedef lstTypes::value_type type;
};
 
int main()
{
  A a;
 
  // je mets une lambda, histoire de simplifier le code ; on peut passer
  // n'importe quel callable, que ce soit un foncteur, un boost.function,
  // etc.
  a.foreach([](A::type& v){ v.bar(); });
}

[koala01]

De plus, si "volaille","cochon","canard" et "poulet" héritent, de manière directe ou inidirecte de "animal", on peut clairement estimer que les tableaux correspondant ne sont là que pour te permettre de garder une certaine séparation des différents éléments si, d'aventure, tu as besoin d'un type d'animal donné, de manière à ne pas te trouver face à une situation dans laquelle tu devrais commencer à vérifier le type réel de tous les animaux lorsque tu ne voudrais travailler que sur un type particulier.


Nous en revenons donc bel et bien dans le cas où les différents tableaux (hormis le tableau d'animaux) ne sont là que... pour permettre à la classe de rendre les services que l'on attend d'elle. Et cela ne signifie pas forcément renvoyer ces tableaux

Je te concède que c'est également le cas avec les itérateurs pour autant, on a toujours des limites au fait de cacher la représentation interne, avec un itérateur (ou un sub_range): si on change la représentation interne de la collection des MonType d'un vector à une map, le code client sera impacté (on ne peut plus simplement déréférencer l'itérateur pour accéder à l'élément, on est obligé de faire un truc style it->second).

Tout à fait, il y a des limites...

Mais elles sont malgré tout moins restreintes dés que tu veilles à travailler avec des itérateurs et à garder les garder compatibles

Sans compter le fait que, si tu en viens à remplacer un std::vector (ou tout autre collection présentant des itérateurs compatibles) par une std::map, c'est sans doute que tu redéfinis en profondeur la responsabilité de ta classe et qu'elle nécessitera de toutes manières un remaniement en profondeur

[JolyLoic]

Ça me rappelle un bouquin (de Meyer il me semble, genre Effective STL) qui indiquait qu'il y a des limites à l'interchangeabilité des types de conteneurs et que ce n'est pas non plus ce qu'il fallait forcément chercher dans nos codes.

Il en parle dedans, en effet. Le cas où l'interface est différente n'est peut-être pas le pire. Il y a aussi le cas où tout compile, tout marche, on a simplement introduit un algorithme d'une complexité absurde.

Enfin dernier point, malgré le SRP, il m'arrive d'avoir à gérer au sein d'une même classe, plusieurs collection (c'est plutôt l'exception mais ça m'arrive).

Ça m'arrive aussi. Par exemple, dans une classe de graphes, avoir dans une classe Nœud un conteneur pour les nœuds précédents, et un pour les nœuds suivants n'est pas absurde.

L'intérêt d'encapsuler un vecteur, c'est de permettre d'effectuer des traitements plus spécialisés sur ce vecteur. Dans on exemple, il eut été plus simple de proposer un méthode foreach() dans ta classe (prenant par exemple un foncteur en paramètre) plutot que d'exposer la collection interne.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
 
struct A // la même que ta classe, plus : 
{
public:
  template <class Callable> void foreach(Callable callable)
  {
    std::for_each(mes_types_m.begin(), mes_types_m.end(), callable);
  }
  typedef lstTypes::value_type type;
};
 
int main()
{
  A a;
 
  // je mets une lambda, histoire de simplifier le code ; on peut passer
  // n'importe quel callable, que ce soit un foncteur, un boost.function,
  // etc.
  a.foreach([](A::type& v){ v.bar(); });
}

Bof... On permet de faire un for_each, mais avec une syntaxe assez lourde, et on empêche de faire plein d'autres choses qui seraient valides aussi. Ne serait-ce qu'exécuter le même for-each en parallèle (ce qui est un choix que doit effectuer le client de la classe, et non son concepteur, car lui seul sait si les traitements sont compatibles avec le parallélisme.

Et même si on voulait ne permettre qu'un for-each, je préfère la syntaxe sans lambdas ni liste de capture :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
for(auto node : A.previousNodes())
{
  // Faire quelquechose
}

En fait, je pense qu'il y a des classes pour lesquelles dire qu'elle est composé d'une collection de sous objets fait partie de l'interface, qu'on ne sait pas a priori comment ces objets vont être manipulés, et qu'on n'a pas envie de brider l'utilisateur et dans ce cas, autant l'exposer (peut-être juste sous forme de range, si c'est une exposition en lecture seule, sinon directement le conteneur).

Et il y a des classes pour lesquelles le fait de gérer une collection est un détail d'implémentation, et là, d'accord pour ne pas l'exposer et fournir seulement des fonctionnalités de plus haut niveau (et je ne considère pas vraiment le fait d'iterer comme de haut niveau).

Un exemple qui pour moi entre clairement dans la première catégorie, c'est encore une fois ma classe de Nœud dans un graphe. Un exemple de la seconde catégorie serait par exemple une classe gérant des prêts pour une bibliothèque, qui aurait des fonctions EnvoyerLettreDeRappel, AjouterUnLivre...

Il n'est pas innocent que l'un de mes exemples soit une classe technique, et l'autre une classe métier.