pointeurs et références question

Question

#ifndef DELETE 
    #define DELETE(var) delete var, var = NULL 
#endif 

using namespace std; 

class Teste { 
    private: 
     Teste *_Z; 

    public: 
    Teste(){ 
     AnyNum = 5; 
     _Z = NULL; 
    } 
    ~Teste(){ 
     if (_Z != NULL) 
      DELETE(_Z); 
    } 

    Teste *Z(){ 
     _Z = new Teste; 
     return _Z; 
    } 
    void Z(Teste *value){ 
     value->AnyNum = 100; 
     *_Z = *value; 
    } 

    int AnyNum; 
}; 

int main(int argc, char *argv[]){ 
    Teste *b = new Teste, *a; 

    a = b->Z(); 

    cout << "a->AnyNum: " << a->AnyNum << "\n"; 

    b->Z(new Teste); 

    cout << "a->AnyNum: " << a->AnyNum << "\n"; 

    //wdDELETE(a); 
    DELETE(b); 
    return 0; 
} 

Je voudrais savoir s'il y a une fuite de mémoire dans ce code cela fonctionne ok, le *a est défini deux fois et les AnyNum imprime différents numéros sur chaque cout << , mais je me demande ce qui est arrivé à le _Z après le setter (new Teste), je n'ai pas encore beaucoup de connaissances en pointeurs/références, mais pour la logique je suppose qu'il est permuté pour la nouvelle variable s'il fuit, y at-il de toute façon à accomplir cela sans avoir pour définir à nouveau un _Z? parce que l'adresse n'a pas changé, juste la mémoire directe allouée J'allais utiliser *& au lieu de seulement des pointeurs, mais cela ferait-il la différence?

Answer 1

Il y a fuite de mémoire sur cette ligne:

b->Z(new Teste); 

en raison de la définition de f la fonction:

void Z(Teste *value){ 
    value->AnyNum = 100; 
    *_Z = *value; 
} 

Il ressemble à Z sans arguments était censé être un getter et avec des arguments d'un compositeur. Je pense que vous vouliez dire à faire:

void Z(Teste *value){ 
    value->AnyNum = 100; 
    _Z = value; 
} 

(note de la troisième ligne) C'est, attribuer le pointeur « valeur » au pointeur « _Z » au lieu de copier quelle valeur a à plus de ce que Z a à. Avec cela, la première fuite de mémoire serait résolue, mais le code en aurait toujours un puisque _Z aurait pu contenir un pointeur. Donc, vous auriez à faire:

void Z(Teste *value){ 
    value->AnyNum = 100; 
    delete _Z; // you don't have to check for null 
    _Z = value; 
} 

Comme mentionné dans un autre commentaire, la vraie solution est d'utiliser des pointeurs intelligents. Voici une approche plus moderne du même code:

using namespace std; 

class Teste { 
    private: 
     boost::shared_ptr<Teste> Z_; 

    public: 
    Teste() : AnyNum(5), Z_(NULL) 
    { } 

    boost::shared_ptr<Teste> Z() 
    { 
     Z_.reset(new Teste); 
     return Z_; 
    } 

    void Z(boost::shared_ptr<Teste> value) 
    { 
     value->AnyNum = 100; 
     Z_ = value; 
    } 

    int AnyNum; 
}; 

int main(int argc, char *argv[]){ 
    boost::shared_ptr<Teste> b = new Teste, a; 

    a = b->Z(); 

    cout << "a->AnyNum: " << a->AnyNum << "\n"; 

    b->Z(boost::shared_ptr<Teste>(new Teste)); 

    cout << "a->AnyNum: " << a->AnyNum << "\n"; 

    return 0; 
} 

Answer 2

Oui il y a:

void Z(Teste *value) 
{ 
    value->AnyNum = 100; 
    *_Z = *value; // you need assignment operator 
} 

L'opérateur d'affectation généré par le compilateur ne fera pas une copie en profondeur, au lieu qu'il fera une copie superficielle. Ce que vous avez à faire est d'écrire un opérateur d'affectation approprié (et éventuellement un constructeur de copie) pour Teste. En outre, vous ne devez pas vérifier si un pointeur est NULL avant de le supprimer:

~Teste() 
{ 
    // no need for checking. Nothing will happen if you delete a NULL pointer 
    if (_Z != NULL) 
    DELETE(_Z); 
} 

Answer 3

Vous avez un autre problème: _Z n'est pas un identifiant que vous devriez utiliser. En général, il est préférable d'éviter les traits de soulignement, et en particulier les doubles soulignements ou traits de soulignement suivis d'une lettre majuscule sont réservés à la mise en œuvre.

Answer 4

(j'ai essayé d'ajouter ceci comme commentaire mais vis le code ..)

J'ASLO suggère fortement de ne pas utiliser

#ifndef DELETE 
    #define DELETE(var) delete var, var = NULL 
#endif 

mais plutôt quelque chose comme

struct Deleter 
{ 
    template< class tType > 
    void operator() (tType*& p) 
    { 
    delete p; 
    p = 0; 
    } 
}; 

utilisation:

Deleter()(somePointerToDeleteAndSetToZero); 

Answer 5

void Z (Valeur de Teste *) { value-> AnyNum = 100; * _Z = * valeur; }

et

b->Z(new Teste); 

crée une fuite de mémoire

la 'nouvelle Teste' n'est supprimé, au lieu de ce que vous faites est l'attribution d'un nouvel objet en tant que paramètre, puis copie tout ce qui s'y trouve en utilisant la valeur * _Z = * mais l'objet n'est pas supprimé après l'appel.

si vous deviez écrire

Test* param - new Teste; 
b->Z(param) 
delete param; 

il serait préférable

bien sûr le plus emploierait boost :: shared_ptr ou quelque chose de similaire pour éviter les soins et supprimer au sujet de cette

Answer 6

Quel désordre! L'ensemble du programme est très difficile à lire en raison du choix des noms d'identification pour commencer:

#ifndef DELETE 
    #define DELETE(var) delete var, var = NULL 
#endif 

Je trouve cela très laid. Lors de l'utilisation des classes, il semble très un-nessacery. Vous pouvez l'utiliser là où une variable est hors de portée mais c'est un gaspillage de temps dans le destructeur. Je pense que ce serait Asier d'envelopper le code dans un pointeur intelligent:

---

class Teste 
{ 
    private: 
     Teste *_Z; 

    public: 
     Teste() 
     ~Teste() // Delete the _Z pointer. 
     Teste *Z(); 
     void Z(Teste *value); 
}; 

Ok. Vous avez un membre de pointeur que vous supprimez dans le destructeur. Cela signifie que vous prenez possession du pointeur. Cela signifie que l'ule de quatre s'applique (semblable à la règle de trois mais applicable aux règles de propriété). Cela signifie que vous devez écrire 4 méthodes ou que les versions générées par le compilateur gâcheront votre code. Les méthodes que vous devez écrire sont:

A Normal (or default constructor) 
A Copy constructor 
An Assignment operator 
A destructor. 

Votre code n'en a que deux. Vous devez écrire les deux autres. Ou votre objet ne doit pas devenir propriétaire du pointeur RAW. c'est à dire. utilisez un Smart Pointer.

---

Teste *_Z; 

Ce n'est pas autorisé. Les identifiants commençant par un tiret bas et une lettre majuscule sont réservés. Vous courez le risque de voir une macro de système d'exploitation corrompre votre code. Arrêtez d'utiliser un trait de soulignement comme premier caractère des identifiants.

---

~Teste(){ 
    if (_Z != NULL) 
      DELETE(_Z); 
} 

Ce n'est pas nécessaire. Asimple supprimer _Z aurait été bien. _Z est hors de portée car il est dans le destructeur, donc pas besoin de le mettre à NULL. L'opérateur delete gère correctement les pointeurs NULL.

~Test() 
{ delete _Z; 
} 

---

Teste *Z(){ 
    _Z = new Teste; 
    return _Z; 
} 

Qu'est-ce qui se passe si vous appelez Z() plusieurs fois (PS mettre le * à côté du Z plutôt que à côté de la rendre difficile Teste à lire). Chaque fois que vous appelez Z(), la variable membre _Z reçoit une nouvelle valeur. Mais qu'arrive-t-il à l'ancienne valeur? Fondamentalement, vous le fuyez. En retournant un pointeur sur un objet appartenant à dans Teste, vous donnez à quelqu'un d'autre la possibilité d'abuser de l'objet (supprimez-le etc). Ce n'est pas bien. Il n'y a pas de propriété claire indiquée par cette méthode.

Teste& Z() 
{ 
    delete _Z;  // Destroy the old value 
    _Z = new Teste; // Allocate a new value. 
    return *_Z;  // Return a reference. This indicates you are retaining ownership. 
        // Thus any user is not allowed to delete it. 
        // Also you should note in the docs that it is only valid 
        // until the next not const call on the object 
} 

---

void Z(Teste *value){ 
    value->AnyNum = 100; 
    *_Z = *value; 
} 

Vous copiez le contenu d'un objet nouvellement construit (qui contient un pointeur) dans un autre objet créé dynamiquement! Que se passe-t-il si _Z n'a pas été attribué en premier. Le constructeur le définit à NULL donc il n'y a aucune garantie qu'il a une valeur valide. Tout objet que vous allouez doit également être supprimé. Mais ici la valeur est allouée dynamiquement passée en Z mais jamais libérée. La raison pour laquelle vous vous en débarrassez est que le pointeur est c activé dans _Z et que _Z est supprimé lorsque son destructeur est détruit.

---

Teste *b = new Teste, *a; 

qui est vraiment entendu lire. Don; t être paresseux l'écrire correctement. Ceci est considéré comme un mauvais style et vous ne pourriez jamais passer en revue le code avec cela.

Teste* b = new Teste; 
Teste* a; // Why not set it to NULL 

---

a = b->Z(); 

Obtenir objet ab pour un. Mais qui détruisait l'objet a ou b?

b->Z(new Teste); 

Cela devient juste trop compliqué après cela.

Answer 7

(pas vraiment une réponse, mais un commentaire ne ferait pas)

La façon dont vous définissez votre macro est sujette à une erreur subtiles (et le fait que personne ne repéré jusqu'à présent juste le prouve).Considérez votre code:

if (_Z != NULL) // yes, this check is not needed, but that's not the point I'm trying to make 
       DELETE(_Z); 

Que se passe-après le préprocesseur passe:

if (_Z != 0) 
     delete _Z; _Z = 0; 

Si vous avez encore du mal à le voir, laissez-moi indenter correctement:

if (_Z != 0) 
     delete _Z; 
_Z = 0; 

Ce n'est pas un grand affaire, compte tenu de cette condition si particulier, mais il va exploser avec toute autre chose et vous passerez âges en essayant de comprendre pourquoi vos pointeurs sont su ddenly NULL. C'est pourquoi les fonctions en ligne sont préférées aux macros - il est plus difficile de les gâcher. Edit: ok, vous avez utilisé la virgule dans votre définition de macro pour être en sécurité ... mais je dirais quand même qu'il est plus sûr d'utiliser la fonction [inline] dans ce cas. Je ne suis pas un des mecs qui n'utilisent pas les macros, mais je ne les utiliserais pas à moins qu'ils ne soient strictement nécessaires et ils ne sont pas dans ce cas