Discussion :

[tails]

Bonjour à tous

J'ai besoin de créer un constructeur par copie dans une classe personnelle, et deux opérateurs d'affectations (un avec une R-Value), mais j'ai dû dupliquer le code :

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
 
Operation::Operation(const Operation &valueToCopy){
    this->_operator = valueToCopy._operator;
    this->_tile_1 = valueToCopy._tile_1;
    this->_tile_2 = valueToCopy._tile_2;
}
 
Operation& Operation::operator=(Operation&& valueToCopy){
    this->_operator = valueToCopy._operator;
    this->_tile_1 = valueToCopy._tile_1;
    this->_tile_2 = valueToCopy._tile_2;
    return *this;
}
 
Operation& Operation::operator=(const Operation& valueToCopy){
    this->_operator = valueToCopy._operator;
    this->_tile_1 = valueToCopy._tile_1;
    this->_tile_2 = valueToCopy._tile_2;
    return *this;
}

(Si vous vous demandez pourquoi les 3, c'est simplement pour mieux utiliser la STL, elles paraissent indispensables pour utiliser les méthodes de certaines collections).

Comment faire plus simple, factoriser tout cela en réutilisant l'une des 3 méthodes dans les deux autres ? J'avais vu dans un cours, mais j'ai carrément oublié.

Autre question liée : vu que je n'utilise pas de pointeur nu dans ma classe, je n'ai pas besoin de surcharger le destructeur : n'est-ce pas ?

[Médinoc]

Utiliser l’idiome copy-and-swap (ou move-and-swap pour le déplacement).

[Bousk]

Comment faire plus simple, factoriser tout cela en réutilisant l'une des 3 méthodes dans les deux autres ?

Par exemple

Code :

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

1
2
3
4
5
6
7
8
struct A
{
 A();
 A(const A& a) { *this = a; }
 A& operator=(const A&) { /* copie des membres */ return *this; }
 A(A&& a) { *this = std::move(a); }
 A& operator=(A&& a) { /* déplacement des membres */ return *this; }
};

Fournir l'opérateur de déplacement sans le constructeur de déplacement est pour le moins étrange.

[ternel]

Normalement, on procède dans l'autre sens, en définissant l'opérateur d'après le constructeur.
C'est le fameux "copy and swap", qui décrit le contenu de l'opérateur d'assignation.
L'idée de base, c'est de créer des constructeurs propres, et de passer par swap, une fonctionalité généralement appréciable.

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
struct A {
    int membre;
 
    // un constructeur "normal"
    A(int value): membre(value) { /* généralement rien */}
 
    A(const A& a): membre(a.membre) { /* généralement rien de plus que le constructeur normal */ }
    A(A&& a): membre( std::move(a.membre) ) { /* généralement rien de plus que le constructeur normal */}
 
    A& operator =(A a) { swap(a); return *this;}
 
    void swap(A & other) {
        using std::swap;
        swap(membre, other.membre);
    }
};

L'intérêt de ce schéma, c'est qu'il n'y a qu'un opérateur d'assignation, qu'il couvre copie et déplacement.
Le gain "au passage" est la capacité de swap.

la documentation sur cppreference est intéressante (mais rangée dans plusieurs pages). Point d'entrée: la surcharge de operator=.

J'ajoute que je n'ai pas déterminé si le std::move dans le constructeur par déplacement est valable, utile ou requis

[tails]

Merci pour vos réponses

Je reprendrais tout cela à tête reposée, histoire de ne pas faire de bêtise lors de l'adaptation.

[koala01]

Salut,

De plus, comme tu te retrouves finalement dans le "cas de base" de chacun des constructeurs, tu peux normalement te contenter de les déclarer "default" (si ton compilateur le supporte: certaines version de Visual Studio ne le font pas ).

Je m'explique:

• Le constructeur par défaut utilise le constructeur par défaut des membres de la classe ==> il n'y a don rien à faire de plus : on peut le déclarer default
• le constructeur de copie (MaClasse (MaClasse const &)) appelle le constructeur de copie des membres de la classe ==> il n'y a don rien à faire de plus : on peut le déclarer default
• le constructeur de copie par déplacement (MaClass(MaClasse &&)) appelle le constructeur de copie par déplacement des membres de la classe ==> il n'y a don rien à faire de plus : on peut le déclarer default
• l'opérateur d'affectation (MaClasse & operator= (MaClasse const &)) appelle l'opérateur d'affectation des membres de la classe ==> il n'y a don rien à faire de plus : on peut le déclarer default
• l'opérateur d'affectation par déplacement (MaClasse & operator= (MaClasse &&)) appelle l'opérateur d'affectation par déplacement des membres de la classe ==> il n'y a don rien à faire de plus : on peut le déclarer default

En plus, toutes ces fonctions (car ne nous y trompons pas, ce sont bien des fonctions, même si elles sont "particulières") sont automatiquement créées par le compilateur... ==> A moins d'avoir une bonne raison de faire autrement (Grande règle des trois/des cinq, peut-être), tu n'a a priori aucune raison ni de les déclarer, ni de les implémenter toi-même

Rappel : Si ta classe intervient dans une hiérarchie de classes (vu que tu l'appelle operation, on peut estimer que c'est le cas), ta classe a sémantique d'entité, et elle ne devrait donc être ni copiable, ni assignable d'aucune manière

[dalfab]

Je ne vois pas du tout l’intérêt de ces fonctions qui ne font que ce que font les fonctions par défaut. Mais en nettement moins optimal!
Voilà le code qui est utilisé si ces fonctions ne sont jamais déclarés ou si elles sont définies en "= default" :

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
Operation::Operation( const Operation &valueToCopy )
:  _operator(valueToCopy._operator),
   _tile_1(valueToCopy._tile_1) ,  _tile_2(valueToCopy._tile_2) {
       // crée les 3 membres et les initialise en même temps
}
 
Operation& Operation::operator=( Operation&& valueToCopy ) {
   this->_operator = std::move(valueToCopy._operator);
   this->_tile_1 = std::move(valueToCopy._tile_1);
   this->_tile_2 = std::move(valueToCopy._tile_2);
   // fait un transfert des éléments au lieu d'une simple copie
   return *this;
}
 
Operation& Operation::operator=( const Operation &valueToCopy ) {
    // peut utiliser un swap par exemple ou
   if ( &valueToCopy != this ) {
      this->_operator = valueToCopy._operator;
      this->_tile_1 = valueToCopy._tile_1;
      this->_tile_2 = valueToCopy._tile_2;
   }
   return *this;
}
 
// et il manque une quatrième, le constructeur par transfert
Operation::Operation( Operation&& valueToMove )
:  _operator(std::move(valueToMove._operator)),
   _tile_1(std::move(valueToMove._tile_1)) ,  _tile_2(std::move(valueToMove._tile_2) {
}

[tails]

Merci beaucoup

Cette réponse m'a l'air la plus adaptée