Discussion :

[LinuxUser]

Bonjour,

J'aimerai avoir vos avis sur l'exemple suivant :

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
 
Classe C
{
  int m_x;
  int m_y;
  int m_z;
 
  public :
    C(int x, int y, int z)
    {
       m_x = x;
       m_y = y;
       m_z = z;
    }
}
 
main.cpp :
...
C c = C(3, 6, 8);
...
 
while(...)
{
  val_x = ...;
  val_y = ...;
  val_z = ...;
 
  c = C(val_x, val_y, val_z);
 ...
}
 
...

Outre le fait que le constructeur soit utilisé comme un gros setter(), n'y a-t-il pas de risque ou de problème à appeler plusieurs fois le constructeur sur le même objet, qui plus est dans un while ?

Merci de votre aide.

[jabbounet]

Dans un code du genre (attention je ne l'ai pas compilé):

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
int x = 0;
int y = 1000;
int z = 50;
 
while (x < y)
{
     C locale(x,y,z);
     C* pasLocale = new C(x*2,y*2,z*2);
 
     x +=z;
     y -= z;
}

Ajoute une trace (std::cout) dans les constructeurs/destructeurs de la classe C et tu verra ce qu'il se passe.
Autre solution utiliser le débugger.

La manipulation est simple a faire et l'expérimentation aide beaucoup pour apprendre et retenir.

Une notion fondamentale a regarder en C/C++ sont la notion de pile et de tas ainsi que la durée de vie/portée des variables/objets

[Bousk]

Bonjour,

de quel risque parles-tu ?
Il n'y a aucun risque, oui les constructeurs/destructeurs sont souvent appelés, mais est-ce dérangeant ? Pourquoi ?

D'ailleurs, ce n'est pas le constructeur qui sera appelé dans la boucle, mais plus surement l'opérateur d'affectation.
Tout comme la première affectation à la déclaration peut utiliser le constructeur par copie.

[LinuxUser]

Ajoute une trace (std::cout) dans les constructeurs/destructeurs de la classe C et tu verra ce qu'il se passe.
Autre solution utiliser le débugger.

Je l'ai fait, et à chaque tour de boucle, le constructeur est appelé deux fois (comme prévu).

La manipulation est simple a faire et l'expérimentation aide beaucoup pour apprendre et retenir.

Je vois pas bien où tu veux en venir, désolé.

[LinuxUser]

Bonjour,

de quel risque parles-tu ?
Il n'y a aucun risque, oui les constructeurs/destructeurs sont souvent appelés, mais est-ce dérangeant ? Pourquoi ?

D'ailleurs, ce n'est pas le constructeur qui sera appelé dans la boucle, mais plus surement l'opérateur d'affectation.
Tout comme la première affectation à la déclaration peut utiliser le constructeur par copie.

Je parle de risque de fuite de mémoire par exemple, et même si ce n'est pas le cas peut-être, je trouve que ce n'est pas une bonne pratique d'utiliser le constructeur comme setter() et l'appeler en boucle de la sorte.

Suis-je le seul à trouver que ce n'est pas une bonne pratique?

[Neckara]

Le constructeur est fait pour initialiser les attributs.
Soit il les initialise lui-même, soit il appelle des setters donc je ne vois pas où est le problème.

Dans la boucle :
C locale(x,y,z);Créera un objet différent à chaque tour et sera détruit en fin de tour de boucle.
Pour C* pasLocale = new C(x*2,y*2,z*2);On aura en effet une fuite de mémoire car il n'y a pas de delete, mais ça n'a rien à voir avec le constructeur.

Il peut en effet y avoir une fuite de mémoire dans un constructeur mais il sera dû à un delete manquant dans le destructeur.

Le constructeur n'est appelé qu'une seule fois lors de la création de l'objet.

C'est ce que jabbounet voulait te montrer avec son exemple.

[Bousk]

Je parle de risque de fuite de mémoire par exemple, et même si ce n'est pas le cas peut-être, je trouve que ce n'est pas une bonne pratique d'utiliser le constructeur comme setter() et l'appeler en boucle de la sorte.

Suis-je le seul à trouver que ce n'est pas une bonne pratique?

Une fuite mémoire n'est pas lié à un constructeur mais imputable au développeur qui a fait de la merde. Que ce soit un constructeur pourri, ou toute autre méthode qui ne prend pas en compte tous les paramètres (allouer un membre ? cas où il est déjà alloué ? le supprimer pour le réallouer ? ... ?).
Les bonnes pratiques, comme leur nom l'indique, ce sont des pratiques. Et rien ni personne ne peut empêcher quelqu'un de faire de la merde et d'aller à leur encontre.

De plus dans ce cas, très/trop simple, il n'y a aucune mémoire gérée dynamiquement, et ce sont plus surement les opérateurs d'affectation et constructeur de copie qui sont utilisés.
Le seul constructeur vraiment utilisé est pour créer l'objet temporaire C(val_x, val_y, val_z) dans la boucle.
c est créé en amont de la boucle.

Le constructeur devrait utiliser les liste d'initialisation.
Il faudrait ajouter le constructeur par copie et l'opérateur d'affectation. Ceux par défaut font l'affaire mais ça permet de savoir par où l'on passe.

[cob59]

Ajoute un setter :

Code :

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

1
2
3
4
5
6
7
8
9
while(...)
{
  val_x = ...;
  val_y = ...;
  val_z = ...;
 
  c.setCoordinates(val_x, val_y, val_z);
 ...
}

Ou met tes attributs en public (je risque de faire jaser, là) :

Code :

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

1
2
3
4
5
6
while(...)
{
  c.x = ...;
  c.y = ...;
  c.z = ...;
}

Mais c'est vrai que créer un objet temporaire pour l'affecter à c n'est pas une bonne méthode. Sans parler des performances, si ton code évolue et que tu décides d'ajouter un attribut w à ta classe C, tu risques d'écraser w alors que le code original n'est censé modifier que x, y et z. Sans parler du chaos que w engendrerait dans le cas où ce serait un pointeur et où C::operator=(const C&) ne serait pas méticuleusement redéfini.

[LinuxUser]

Ajoute un setter :

Code :

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

1
2
3
4
5
6
7
8
9
while(...)
{
  val_x = ...;
  val_y = ...;
  val_z = ...;
 
  c.setCoordinates(val_x, val_y, val_z);
 ...
}

Oui, c'est ce à quoi j'avais pensé.

Ajoute un setter :
Ou met tes attributs en public (je risque de faire jaser, là) :

Code :

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

1
2
3
4
5
6
while(...)
{
  c.x = ...;
  c.y = ...;
  c.z = ...;
}

Nan là quand même pas.

Ajoute un setter :
Mais c'est vrai que créer un objet temporaire pour l'affecter à c n'est pas une bonne méthode. Sans parler des performances, si ton code évolue et que tu décides d'ajouter un attribut w à ta classe C, tu risques d'écraser w alors que le code original n'est censé modifier que x, y et z. Sans parler du chaos que w engendrerait dans le cas où ce serait un pointeur et où C::operator=(const C&) ne serait pas méticuleusement redéfini.

Surtout que je récupère et découvre un code existant et justement j'essaye de faire les choses proprement pour le faire évoluer et ensuite le maintenir.

En tout cas merci beaucoup à tous.

[screetch]

aucun d'entre vous n'a lu la question correctement et n'a repondu correctement... (sauf Bousk)
LinuxUser, n'ajoute pas de setter, le code que tu as fait est parfaitement valide.

a chaque tour de boucle, le compilateur C++ va etendre le code que tu as montre:

Code :

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

c = C(val_x, val_y, val_z);

pour le "remplacer" par:

Code :

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

1
2
3
C temp = C(val_x, val_y, val_z);
c.operator=(temp);
temp.~C();

c'est a dire qu'il ne va pas ecraser la variable c par une nouvelle valeur comme ca, il va construire un temporaire puis utiliser un operateur C++ special que le compilateur peut generer automatiquement (mais parfois, pas parfaitement), et qui consiste a remplacer (proprement) les valeurs contenues dans c par les valeurs contenues dans temp, puis va detruire temp qui n'est plus utile.

regarde dans la FAQ les constructeurs de copie, les operateurs d'affectation et les destructeurs; ces methodes sont speciales et sont mises en jeu dans le code exemple que tu montres.

[LinuxUser]

a chaque tour de boucle, le compilateur C++ va etendre le code que tu as montre:

Code :

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

c = C(val_x, val_y, val_z);

pour le "remplacer" par:

Code :

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

1
2
3
C temp = C(val_x, val_y, val_z);
c.operator=(temp);
temp.~C();

c'est a dire qu'il ne va pas ecraser la variable c par une nouvelle valeur comme ca, il va construire un temporaire puis utiliser un operateur C++ special que le compilateur peut generer automatiquement (mais parfois, pas parfaitement), et qui consiste a remplacer (proprement) les valeurs contenues dans c par les valeurs contenues dans temp, puis va detruire temp qui n'est plus utile.

C'est ce que j'ai cru comprendre, mais j'avoue que je ne savais que cela fonctionnait ainsi.

regarde dans la FAQ les constructeurs de copie, les operateurs d'affectation et les destructeurs; ces methodes sont speciales et sont mises en jeu dans le code exemple que tu montres.

Oui, c'est ce que je vais faire.

LinuxUser, n'ajoute pas de setter, le code que tu as fait est parfaitement valide.

Ce n'est pas nécessaire de remplacer par un setter() et tu me déconseilles de le faire?

[screetch]

franchement, si c'etait moi, pour l'exemple que tu donnes (mais c'est que pour l'exemple que tu donnes):

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
struct C
{
  int x;
  int y;
  int z;
}
 
 
C c = { 3, 6, 8 };
...
 
while(...)
{
  val_x = ...;
  val_y = ...;
  val_z = ...;
 
  c = C c = C(3, 6, 8);
...
 
while(...)
{
  val_x = ...;
  val_y = ...;
  val_z = ...;
 
  c = { val_x, val_y, val_z }; // en c++ 0X seulement, compilateurs depuis 2009/2010 peut-etre
  c.x = val_x;
  c.y = val_y;
  c.z = val_z; // en C++ normal
 ...
}

il y a d'autres moyens (et un setter, aussi, au pire) mais pour moi il n'y a pas besoin d'encapsuler ces donnees. Mais c'est specifique a cet exemple, sinon je recommande contructeur + copie + affectation + destructeur fait proprement.

[LinuxUser]

OK, et si C était un membre d'une autre classe, ça donnerai quelque chose comme ça :

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
 
class A
{
   C m_c;
}
...
A::A() : m_c(3, 6, 8)
{
  ...
}
 
C A::getC()
{
  return m_c;
}
 
void A::setC(C c)
{
  m_c = c;
}

Main.cpp :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
 
...
while(...)
{
  val_x = ...;
  val_y = ...;
  val_z = ...;
 
   a.setC(C(val_x, val_y, val_z));
 ...
}
...

Dans ce cas, faudrait remplacer C(val_x, val_y, val_z) par un setter() ?

[Bousk]

La ça commence à faire beaucoup, et le code n'est pas top.
Viens-tu du monde java ?
Auquel cas, en C++, passer un objet par référence n'est pas automatique, et chacune de tes fonctions utilise une copie. Et qui dit copie dit constructeur par copie, puis destructeur de la copie.

=> la const référence c'est le bien !

sinon, tu as aussi l'option de montrer le vrai code et ce que tu attends une bonne fois pour toutes.

[LinuxUser]

Oui j'ai appris (cours CPOO + TD/TP) le Java à la Fac, par contre j'ai du faire du C++ sans l'avoir vraiment étudié.

Là je récupère du code C++, qui plus est, est codé comme du C.

Bref, j'essaie juste de faire les choses proprement et conformément aux bonnes pratiques.

[screetch]

pour etre franc, dans ce cas precis, ce sont plutot des details, ce n'est pas franchement a s'arracher les cheveux.

[LinuxUser]

Quelle manière de faire te semble la meilleure ?
Surtout si on a :

Code :

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

1
2
3
4
5
6
7
8
 
class A
{
  C1 m_c1;
  C2 m_c2;
  C3 m_c3;
...
}

main() :

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
 
 
...
while(...)
{
  ...
 
   a.setC1(C1(...));
   a.setC2(C2(...));
   a.setC3(C3(...));
 ...
// et eventuellement à nouveau
a.setC1(C1(...));
...
a.setC3(C3(...));
...
}
...

[screetch]

je mettrais simplement des references comme Bousk dit, sinon, setC1(C(1,2,3)) n'est pas si mal.