[C++]Fun with pointers Options

[Lupuss]

Salut les geeks, j'ai remis mes gros doigts dans du C++ et j'ai qq difficultés avec nos amis les pointeurs.

Donc imaginons une classe lambda avec plusieurs variables membres qui sont des pointeurs char* (oui, String tout ça, je sais, mais ce n'est pas le but ici).

Ces pointeurs son mis à null dans les constructeurs, et puis j'appelle les différents setters. Dans les setters de chaque variable de type pointeur, je commence par faire un delete (le pointeur sera toujours valide ou null normalement) puis un new (ce sera plus lisible dans le code je pense).

Un exemple de classe avec deux pointeurs char* (nom et prenom) et deux entiers (age et taille):

CODE

Personne::Personne(){
cerr << "[Personne] Constructeur par défaut" << endl;

nom = NULL;
prenom = NULL;
setNom(NULL);
setPrenom(NULL);
setAge(18);
setTaille(175);
}

un setter typique:

CODE

void Personne::setNom(const char* const nom){
delete this->nom;
//QUESTION: serait-il utile de mettre le pointeur à null ici? des explications?
this->nom = new char[nom ? strlen(nom) : 0 + 1];

if(nom)
strcpy(this->nom, nom);
else
*this->nom = 0;
}

[cben76]

Salut les geeks, j'ai remis mes gros doigts dans du C++ et j'ai qq difficultés avec nos amis les pointeurs.

Plusieurs remarques :

* pourquoi tu fais

CODE

nom = null;
puis
setnom(null)

si tu fais ton nom = null, ton setnom derriere sert a rien.
Et ton delete de ton setnom arrivera sur rien, vu que tu aura deja fait nom=null

Pourquoi, si le nom que tu passes est null, tu fais un *this->nom = 0 et pas = null ?

Moi je ferai ca :

CODE

void Personne::setNom(const char* const nom){

delete this->nom;

if (nom != null)
{
this->nom = new char[strlen(nom)];
strcpy(this->nom, nom);
}
else
{
this->nom = null;
}

}

Ce message a été modifié par cben76 - 5/11/2009, 16:02.

[Tomma]

Bizarre ton code. Un conseil, prends l'habitude de nommer les données membres de ta classe ainsi "m_nomdelavariable". Et fait commencer le nom de tes classes par un C (comme classe). Puis d'autres détails.

Je remets une partie de ton code à jour pour en espérant répondre à ta question (j'ai pas bien compris ce que tu voulais)

Code

void CPersonne::setNom(const char* const pcstrNom)
{
    if (pcstrNom == NULL)
        return;

    if (m_pstrNom != NULL)         // pour éviter de mauvaises surprises
    {
        delete m_pcstrNom;
        m_pstrNom = null;
    }

    m_pstrNom = new char[strlen(pcstrNom)];
    strcpy(m_pstrNom, pcstrNom);
}

edit: Avant qu'on me fasse la remarque, oui je sais le code peut être encore simplifié

Ce message a été modifié par Tomma - 5/11/2009, 16:34.

[Lupuss]

Je vois ce que vous voulez dire, mais dans ce cas je dois changer tous mes getters pour qu'ils prennent en compte le cas d'un pointeur null

[fser]

C'est tordu ton truc ... pourquoi tu crée une personne avec des infos nulles?
si c'est null, tu set juste pas l'attribut.
Et quand bien même, c'est le rôle du constructeur de les mettre à null au début, si besoin il y a, sans passer par les setters!

Fin je pige pas.

[Lupuss]

La personne avec des infos nulles, c'est parce que la classe doit subir toute une série de tests (genre affichage d'un vecteur d'objets mixtes, etc...) dans le cadre d'un dossier. Les setters dans le constructeur, c'était suggéré pour la modularité (genre changer juste le setter au lieu du constructeur par défaut, de celui de copie et celui d'initialisation si on modifie une variable).

[skeetmtp]

Je vais essayer repondre uniquement a la question, meme si j'ai un doute sur la question en elle meme :

1) si la question etait, est-ce qu'il faudrait pas ajouter un this->nom = 0; apres le delete comme ca :

Code

void Personne::setNom(const char* const nom){
     delete this->nom;
     this->nom = 0; //comme ca ??
     this->nom = new char[nom ? strlen(nom) : 0 + 1];

     if(nom)
         strcpy(this->nom, nom);
     else
         *this->nom = 0;
}

clairement non aucun interet.

2) si la question etait, est-ce qu'il faudrait pas laisser le pointeur null au lieu d'allouer une chaine vide comme ca :

Code

void Personne::setNom(const char* const nom){
     delete this->nom;
     if(this->nom)
     {
         this->nom = new char[strlen(nom) + 1];
         strcpy(this->nom, nom);
     }
     else
         this->nom = 0;
}

moi ca me parait plus coherent de remetre le pointeur a null vu que ca le remet dans l'etat ou il etait apres etre passé par le constructeur

[Lupuss]

Je viens d'éplucher le code en revue avec un pote, les erreurs étaient en fait dues à tout autre chose, mais j'en ai profité pour revoir la classe, et je pense que le résultat est plus propre comme cela:

D'abord le constructeur:

CODE

Personne::Personne() : nom(NULL), prenom(NULL), ident(NULL), age(18), taille(175){
cerr << "[Personne] Constructeur par défaut" << endl;

//setNom("xxx");
}

un setter et un getter:

CODE

void Personne::setNom(const char* const nom){
delete[] this->nom;

if(nom){
this->nom = new char[strlen(nom) + 1];
strcpy(this->nom, nom);
}
else
this->nom = NULL;
}

const char* const Personne::getNom() const{
return nom ? nom : "";
}

Voilà, je ne m'amuse plus à allouer un octet pour rien, et le constructeur s'en trouve bien simplifié aussi (vive les initlists). ça vous parait moins horrible?

[Histrion]

Point par point, pour la base :

1/ Une donnée membre pointeur ça s'initialise à NULL dans le constructeur (ou alors avec une donnée pré-établie).

2/ Il faut toujours mettre en place un déstructeur qui fasse le delete() des pointeurs de cette manière :

CODE

if (this->monpointeur != NULL) {
delete(this->monpointeur);
this->monpointeur = NULL;
}

3/ Dans le setter tu dois faire le delete du pointeur s'il est non-nul avant de réaffecter la nouvelle adresse pointée :

CODE

public void setMonpointeur(void* pointeur) {
if (this->monpointeur != NULL) {
delete(this->monpointeur);
}
this->monpointeur = refpointeur;
}

4/ A l'appel du getter toujours vérifier si le pointeur reçu est NULL avant de l'utiliser.

Après, le plus propre c'est de mettre en place des smart pointers (pointeurs intelligents), soit toi-même, soit via une lib (Boost par exemple).

[Trevidos]

Mon petit grain de sel:

delete[] this->nom;

Pas de delete[], tu n'as pas alloué de tableau d'objet mais un tableau de type primitif. En général, le compilateur s'en sort, mais c'est pas beau quand meme.

Ce message a été modifié par Trevidos - 7/11/2009, 17:30.

[Lupuss]

@Histrion: j'ai l'impression que tu n'as pas tenu compte de mon dernier post. Tu verras que pas mal de points ont été corrigés.

@Trevidos: Merci de m'avoir éclairé sur ce point.

[c0unt0]

Pour en rajouter un peu, je trouve que ton getter peut etre dangereux: tu retourne l'adresse d'une constante probablement cree sur la pile (le ""), si l'appelant s'avise de garder le pointeur pour (beaucoup) plus tards, tu risques, selon le compilo et la plateforme, de tomber sur un bout de memoire pas frais.
A ta place, soit je retournerais juste "nom", et c'est a l'appelant de gerer le NULL, soit je retournerais l'adresse d'une constante vide, genre un "static const char[] = """.
Mais en fait je retournerais NULL.

Valavala.

[Lupuss]

Tiens? Je pensais que le "" est un littéral qui n'est pas créé sur la pile mais dans le data segment?! Pour l'appelant, c'est un test fourni par un prof qui gère que dalle, c'est à notre classe de gérer le maximum de cas.

[c0unt0]

Dans la plus part des cas oui, mais sur certaine plateforme douteuses, c'est pas toujours certain: pour utiliser moins de memoire, certaine constantes seront construite sur la pile... Je sais, et oui, ca surprends toujours la premiere fois...

[AnA-l]

Des noms de plateformes douteuses ?

[GloP]

La station de jeu?

[Lupuss]

Ah ben ça Dans mon cas le soft doit juste tourne sur une distrib linux (Ubuntu) et sur un Solaris, mais je prends note de ta remarque.

On en apprend tous les jours...