Discussion :

[gbdivers]

Bonjour à tous

C'est une discussion qui revient régulièrement sur le chat de Developpez.com. Une personne demande comment fait-on pour accéder aux variables membres privées d'une classe et on lui répond de créer des getter et setter. Viens alors un C++ien moyen (c'est-à-dire un casse-pied, en général moi) qui hurle au scandale et sort l'adage bien connu : "les accesseurs, c'est le mal". S'en suit une discussion sur pourquoi les accesseurs sont à éviter, quand j'ai le temps et l'humeur.
Dans ce billet, je vais présenter les problèmes que posent les accesseurs concernant l'exposition des détails d'implémentation.

Les accesseurs et les détails d'implémentation

Que pensez-vous des problèmes exposés dans cet article et des principes de conception objet ?

[koala01]

Salut,

Ce n'est qu'une lecture en diagonale, mais tu pourrais d'ailleurs aussi insister sur le fait que ta classe Point3D présentée dans les premières lignes a, typiquement, sémantique de valeur et que l'idéal serait, au moment où l'on modifie une des coordonnées, de créer un nouvel objet avec les bonnes coordonnées (en gros, que setX(), setY() et setZ() sont encore pis que leurs accesseurs associés)

Petite faute de grammaire en :

C'est un problème que connait connaissent beaucoup de débutants

(ce sont les débutants qui connaissent le problème

Enfin, mais bon, ce n'est qu'un détail permettant d'avoir une erreur de compilation si la spécialisation de gl_trait n'est pas présente, j'aurais définis les traits sous une forme 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
template<class T> 
struct gl_trait ; 
 
template<> 
struct gl_trait<float> { 
  static inline glVertex  (internal x, internal y, internal z) const { glVertex3f(x, y, z); } 
  static inline glNormal  (internal x, internal y, internal z) const { glNormal3f(x, y, z); } 
  static inline glTexCoord(internal x, internal y, internal z) const { glTexCoord3f(x, y, z); } 
}; 
 
template<> 
struct gl_trait<double> { 
  static inline glVertex  (internal x, internal y, internal z) const { glVertex3d(x, y, z); } 
  static inline glNormal  (internal x, internal y, internal z) const { glNormal3d(x, y, z); } 
  static inline glTexCoord(internal x, internal y, internal z) const { glTexCoord3d(x, y, z); } 
};

On perd la valeur par défaut, mais on gagne une erreur de compilation si besoin

[gbdivers]

Pour la sémantique de valeur, j'en parlais initialement, mais j'ai finalement supprimé ce point, pour alléger les explications. Comme je ne fournis pas d'accesseur ni même de constructeur (utilisation des initializer-list) dans les dernières versions, on ne peut pas modifier de point. J'ajoute un commentaire

Pour la faute, merci

Pour la classe de traits, c'est parce que j'aime bien donner le prototype de la classe de traits, pour que celui qui veut spécialiser puisse directement le faire sans aller chercher le code d'une autre spécialisation... mais je peux mettre le code en commentaire

Merci

[cob59]

Je ne suis pas mécontent de lire ça.
J'ai souvent constaté les 2 mêmes écueils chez les développeurs habitués au procédural et qui tentent de faire de l'objet : un bon gros God-Object qui appelle séquentiellement ses propres méthodes, et des get/set sur tout ce qui bouge parce que "ça fait POO".

En fait, dès qu'une classe présente trop de "propriétés" (ie. des attributs public ou quasi-public via get/set), j'ai tendance à la scinder en deux :

• Une classe POD qui décrit des propriétés (donc des attributs publics) mais n'a aucun comportement.
• Une classe qui décrit un comportement et agrège un POD, ou une hiérarchie d'objets basé sur ce POD.

Exemple : des N-uplets de coordonnées qui décrivent des points, que je passe à un afficheur Drawer, qui manipule ces points.

A ce propos, dans tes exemples, je me demande si la méthode draw() ne confie pas des responsabilités inappropriées pour une classe Point ?

[gbdivers]

A ce propos, dans tes exemples, je me demande si la méthode draw() ne confie pas des responsabilités inappropriées pour une classe Point ?

Oui et non. Très clairement, il n'est pas approprié qu'une classe Point soit chargée de se dessiner elle même en plus de ses fonctionnalités classiques... mais j'ai pris soins ici de ne pas lui donner d'autres responsabilité que de se dessiner elle même, donc respect du principe de responsabilité unique

Pour une vraie classe Point3D, il faudrait en effet soit renommer la classe (DrawablePoint ?), soit séparer ce qui correspond réellement à la sémantique d'un point et l'affichage

[Ekleog]

pour ceux, qui comme LittleWhite bloquebloquent sur l'utilisation de glVertex

Dans gl_traits, tu utilises internal comme arguments aux fonctions, et tu l'as mis en commentaire dans la version non spécialisée. D'ailleurs, pourquoi utiliser internal et non directement float/double dans les versions spécialisées ?

Sinon, intéressant !

[gbdivers]

Dans gl_traits, tu utilises internal comme arguments aux fonctions, et tu l'as mis en commentaire dans la version non spécialisée. D'ailleurs, pourquoi utiliser internal et non directement float/double dans les versions spécialisées ?

Sinon, intéressant !

Corrigé
Pour le internal en commentaire, c'est suite à la remarque de koala01, j'ai mis en commentaire trop vite

Pourquoi internal plutôt que T/float/double ? Par fainéantise
Sans typedef, on devrait effectivement utiliser T pour le template non spécialisé et float/double pour les autres

Code :

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

1
2
3
4
5
6
7
8
9
template<class T = float> 
struct gl_trait { 
  // static inline glVertex  (T x, T y, T z) const {} 
}; 
 
template<> 
struct gl_trait<float> { 
  // static inline glVertex  (float x, float y, float z) const { glVertex3f(x, y, z); } 
};

comme j'écris ma spécialisation en copiant-collant le code de la version non spécialisée, utiliser internal me permet de modifier moins de code (j'ai juste le contenu de la fonction à modifier, pas sa définition)

[oodini]

Ce n'est qu'une lecture en diagonale, mais tu pourrais d'ailleurs aussi insister sur le fait que ta classe Point3D présentée dans les premières lignes a, typiquement, sémantique de valeur et que l'idéal serait, au moment où l'on modifie une des coordonnées, de créer un nouvel objet avec les bonnes coordonnées

Je n'ai absolument jamais vu une telle méthode dans du code 3D. Je n'en vois d'ailleurs pas l'intérêt. Si on a une voiture rouge et qu'on souhaite en changer sa couleur, on ne va pas construire une nouvelle voiture uniquement pour satisfaire ce désir de changement de couleur.

[gbdivers]

Je n'ai absolument jamais vu une telle méthode dans du code 3D. Je n'en vois d'ailleurs pas l'intérêt. Si on a une voiture rouge et qu'on souhaite en changer sa couleur, on ne va pas construire une nouvelle voiture uniquement pour satisfaire ce désir de changement de couleur.

Une classe Voiture aurait clairement une sémantique d'entité et non de valeur
Si tu repeints une voiture, tu as toujours la même voiture. Si tu modifies les coordonnées d'un point, tu as bien un point différents.
Cela à une importance par rapport aux opérateurs et fonctions que l'on peut définir.

Mais il est vrai que l'on rencontre régulièrement du code qui ne respect pas les sémantiques de valeur ou entité (par exemple une classe avec sémantique de valeur et des fonctions virtuelles)

[oodini]

Si tu modifies les coordonnées d'un point, tu as bien un point différents.

Non. Si tu as un polygone constitué de points, que ceux-ci sont référencés sous forme de tableau, et que tu modifie la valeur d'un des points, cela signifie que tu modifies la position du sommet.

La problème de la sémantique, c'est que c'est finalement très subjectif.

[Ekleog]

Mais lorsque tu modifies la position d'un point d'un polygone, tu obtiens un autre polygone.

La question est surtout : où est-ce que ça s'arrête ?

[oodini]

Mais lorsque tu modifies la position d'un point d'un polygone, tu obtiens un autre polygone.

Quand ta voiture, rouge ou bleue, modélisable par un polygone, s'écrase contre un mur, les sommets se déplacent, et c'est pourtant la même voiture.

La question est surtout : où est-ce que ça s'arrête ?

Au moment où la voiture atteint le mur.

[gbdivers]

La problème de la sémantique, c'est que c'est finalement très subjectif.

Bof, pas trop je trouve.

On dit d'une classe qu'elle a une sémantique de valeur si deux objets situés à des adresses différentes, mais au contenu identique, sont considérés égaux.

A l'inverse des classes à sémantique de valeur, une classe a une sémantique d'entité si toutes les instances de cette classe sont nécessairement deux à deux distinctes, même si tous les champs de ces instances sont égaux.

On est bien d'accord qui si tu veux tester si 2 polygones sont identiques, tu ne peux pas te contenter de vérifier qu'il ont la même adresse, mais tu dois vérifier que tous les points sont identiques.
Pour une voiture, même en vérifiant toutes les caractéristiques (modèle, couleur, etc.), ce n'est pas suffisant pour dire que c'est bien la même voiture.
On a bien une sémantique de valeur dans le premier cas et une sémantique d'entité dans le second.

La sémantique d'entité/valeur a un intérêt (ne pas faire n'importe quoi avec les classes), mais comme toutes les règles, on peut ne pas les respecter. Le point important, je pense, est qu'on doit le faire en connaissance de cause, pas par méconnaissance des principes (ce qu'on voit souvent)

Effectivement, il est classique d'avoir un buffer de points en 3D que l'on passe directement au gpu et que donc, pour des raisons de simplification, on va directement modifier les coordonnées des points dans le buffer. Mais c'est clairement un non respect de la sémantique (et du principe d'encapsulation par la même occasion) Et a priori, on pourrait faire correctement en respectant la sémantique (mais je fais pas non plus )

[Ekleog]

On est bien d'accord qui si tu veux tester si 2 polygones sont identiques, tu ne peux pas te contenter de vérifier qu'il ont la même adresse, mais tu dois vérifier que tous les points sont identiques.
Pour une voiture, même en vérifiant toutes les caractéristiques (modèle, couleur, etc.), ce n'est pas suffisant pour dire que c'est bien la même voiture.
On a bien une sémantique de valeur dans le premier cas et une sémantique d'entité dans le second.

Sauf que la voiture a une plaque d'immatriculation, donc bon... ?

Effectivement, il est classique d'avoir un buffer de points en 3D que l'on passe directement au gpu et que donc, pour des raisons de simplification, on va directement modifier les coordonnées des points dans le buffer. Mais c'est clairement un non respect de la sémantique (et du principe d'encapsulation par la même occasion) Et a priori, on pourrait faire correctement en respectant la sémantique (mais je fais pas non plus )

En même temps, le souci vient alors des performances : on perd beaucoup de mémoire et/ou beaucoup de temps de copie.
Parce que quelle est la différence entre p.setX(42); et p = p.withX(42); ?

Enfin, juste pour dire que, àmha, si on a un couple de getter/setter qui n'effectue aucune vérification, autant mettre l'attribut public. Ca respectera aussi peu les principes, mais au moins ça donne du sucre syntaxique.
Donc, la classe Point du début, je l'aurais écrite :

Code :

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

1
2
3
4
template <typename T>
struct point {
   T x, y, z;
};

D'ailleurs, j'aurais certainement gardé cette classe (potentiellement templatisée), et créé une fonction libre draw(point<T>).
Parce que faire une classe de traits / un point_drawer pour ça, ça me semble clairement overkill.
Surtout que la classe de traits n'est pas extensible sans toucher directement à son interface.

[gbdivers]

Sauf que la voiture a une plaque d'immatriculation, donc bon... ?

Je m'attendais à cette remarque
Malheureusement, tu travailles sur le logiciel de vérification des plaques d'immatriculation contrefaites, ce n'est pas suffisant pour valider l'unicité de ta voiture

En même temps, le souci vient alors des performances : on perd beaucoup de mémoire et/ou beaucoup de temps de copie.
Parce que quelle est la différence entre p.setX(42); et p = p.withX(42); ?

Pour les performances, bof, pas sur que l'on ait beaucoup de différence entre ça :

Code :

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

1
2
3
4
5
6
vector<Point3D>::iterator it;
it->setX(1.0);
it->setY(1.0);
it->setY(1.0);
// et
*it = Point3D{ 1.0, 1.0, 1.0 };

Vérification, gcc donne le même code asm pour les 2 cas, le non respect de la sémantique de valeur n'est pas justifiée par les performances

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
26
27
28
29
30
31
32
33
34
35
36
37
38
	movl	.LC0(%rip), %eax
	movl	%eax, -128(%rbp)
	movl	.LC1(%rip), %eax
	movl	%eax, -124(%rbp)
	movl	.LC2(%rip), %eax
	movl	%eax, -120(%rbp)
	movl	.LC3(%rip), %eax
	movl	%eax, -128(%rbp)
	movl	.LC4(%rip), %eax
	movl	%eax, -124(%rbp)
	movl	.LC5(%rip), %eax
	movl	%eax, -120(%rbp)
	movl	$0, %eax
	leave
	.cfi_def_cfa 7, 8
	ret
	.cfi_endproc
.LFE0:
	.size	main, .-main
	.section	.rodata
	.align 4
.LC0:
	.long	1065353216
	.align 4
.LC1:
	.long	1073741824
	.align 4
.LC2:
	.long	1077936128
	.align 4
.LC3:
	.long	1082130432
	.align 4
.LC4:
	.long	1084227584
	.align 4
.LC5:
	.long	1086324736

Enfin, juste pour dire que, àmha, si on a un couple de getter/setter qui n'effectue aucune vérification, autant mettre l'attribut public. Ca respectera aussi peu les principes, mais au moins ça donne du sucre syntaxique.
Donc, la classe Point du début, je l'aurais écrite :

Code :

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

1
2
3
4
template <typename T>
struct point {
   T x, y, z;
};

D'ailleurs, j'aurais certainement gardé cette classe (potentiellement templatisée), et créé une fonction libre draw(point<T>).
Parce que faire une classe de traits / un point_drawer pour ça, ça me semble clairement overkill.

Oui, j'utilise aussi un type POD en général dans ce cas, le principal étant d'avoir un minimum de classe manipulant directement le type POD. Mais c'est plus par paresse (et copié-collé de code ancien) que respect de la sémantique ou pour des problèmes de performances

Surtout que la classe de traits n'est pas extensible sans toucher directement à son interface.

Si justement. Il suffit d'ajouter une spécialisation (ou j'ai pas compris)

[oodini]

On est bien d'accord qui si tu veux tester si 2 polygones sont identiques, tu ne peux pas te contenter de vérifier qu'il ont la même adresse, mais tu dois vérifier que tous les points sont identiques.
Pour une voiture, même en vérifiant toutes les caractéristiques (modèle, couleur, etc.), ce n'est pas suffisant pour dire que c'est bien la même voiture.

Tu peux très bien avoir deux polygones différents, avec les mêmes coordonnées, sans pour autant qu'ils représentent la même entité. C'est par exemple ce qu'il se passe quand on fait du morphing : on fait une copie d'un polygone, polyèdre à l'instant 0, et au cours du temps, les sommets vont se déplacer.

[gbdivers]

Tu peux très bien avoir deux polygones différents, avec les mêmes coordonnées, sans pour autant qu'ils représentent la même entité. C'est par exemple ce qu'il se passe quand on fait du morphing : on fait une copie d'un polygone, polyèdre à l'instant 0, et au cours du temps, les sommets vont se déplacer.

Mathématiquement, ça sera le même polygone. Le fais que le même polygone soit contenu dans 2 variables distincts ne change rien.

On dit d'une classe qu'elle a une sémantique de valeur si deux objets situés à des adresses différentes, mais au contenu identique, sont considérés égaux.

[oodini]

Mathématiquement, oui, mais dans la modélisation de ton application, non.

Et lorsque tu déformes un polyèdre de dizaine de milliers de sommets dans une animation 3D, tu comptes reconstruire un objet à chaque instant de l'animation ?

[gbdivers]

Mathématiquement, oui, mais dans la modélisation de ton application, non.

Ca, c'est un détail d'implémentation. Rien n'oblige à réaliser une copie de ton polygone dans la seconde variable. Par exemple, dans Qt, tant que tu ne modifies pas le polygone, c'est une simple redirection vers le premier (COW)

Et lorsque tu déformes un polyèdre de dizaine de milliers de sommets dans une animation 3D, tu comptes reconstruire un objet à chaque instant de l'animation ?

J'ai ajouté dans mon post précédant le code asm généré avec la sémantique d'entité et sans. Au niveau performance, rien ne justifie d'accès direct aux composantes des points par rapport à la sémantique de valeur, le code généré est le même

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
int main() {
    struct Point3D { float x, y, z; };
    Point3D v[10];
 
    // méthode 1
    v[0].x = 1.0;
    v[0].y = 2.0;
    v[0].z = 3.0;
 
    // méthode 2
    v[0] = Point3D { 4.0, 5.0, 6.0 };
}

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
26
27
28
29
30
31
32
33
34
35
36
37
38
	movl	.LC0(%rip), %eax
	movl	%eax, -128(%rbp)
	movl	.LC1(%rip), %eax
	movl	%eax, -124(%rbp)
	movl	.LC2(%rip), %eax
	movl	%eax, -120(%rbp)
	movl	.LC3(%rip), %eax
	movl	%eax, -128(%rbp)
	movl	.LC4(%rip), %eax
	movl	%eax, -124(%rbp)
	movl	.LC5(%rip), %eax
	movl	%eax, -120(%rbp)
	movl	$0, %eax
	leave
	.cfi_def_cfa 7, 8
	ret
	.cfi_endproc
.LFE0:
	.size	main, .-main
	.section	.rodata
	.align 4
.LC0:
	.long	1065353216
	.align 4
.LC1:
	.long	1073741824
	.align 4
.LC2:
	.long	1077936128
	.align 4
.LC3:
	.long	1082130432
	.align 4
.LC4:
	.long	1084227584
	.align 4
.LC5:
	.long	1086324736

[Ekleog]

Pour les performances, bof, pas sur que l'on ait beaucoup de différence entre ça :

Code :

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

1
2
3
4
5
6
vector<Point3D>::iterator it;
it->setX(1.0);
it->setY(1.0);
it->setY(1.0);
// et
*it = Point3D{ 1.0, 1.0, 1.0 };

Vérification, gcc donne le même code asm pour les 2 cas, le non respect de la sémantique de valeur n'est pas justifiée par les performances

Code :

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

[...]

Sauf que là, tu modifies les trois valeurs.
Sur un code comme le suivant, je pense qu'il va y avoir une différence (pas le temps de tester) :

Code :

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

1
2
3
4
5
6
point p{1., 1., 1.};
if (user_controlled_value) { // Eviter constant propagation/folding
  p.setX(42.);
  // Ou
  p = p.withX(42.);
}

Si justement. Il suffit d'ajouter une spécialisation (ou j'ai pas compris)

Non, je parlais d'ajouter une fonction supplémentaire, pas un type supplémentaire. Ajouter une fonction de plus dans la classe de trait obligerait à recompiler tout ce qui dépend de la classe, alors qu'il n'y a aucune raison de lier les fonctions entre elles. Si ?