Discussion :

[deubelte]

Bonjour
J'ai un morceau de code assez étrange:

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
void UneFonction (const MaClasse& IT_member) {
        if (isSet && (__d != ::VSIVOTKind_float))
        {
            this-> VSIVOT::~VSIVOT();
            memset(this, 0, sizeof(*this));
        }
__d = ::VSIVOTKind_frabasis;
 
     if ( !isSet )
        {
            _uneVariable_ = new MaClasse;
            isSet = 1;
        }
 
*(_uneVariable_) = IT_member;

En gros, il appelle lui même son propre destructeur, puis il appelle la fonction memset, qui fixe à 0 la plage libérée. OK.

Ensuite, il se reconstruit à la volée.

C'est assez rare de voir çà. Qu'en pensez vous? Ce code a été fait à mon avis par quelqu'un d'expérimenté, donc je ne critique pas à priori.
Mais j'aimerais savoir ce que vous en pensez, quels sont les cas dans lesquels on a besoin de faire ca.

merci

[DonQuiche]

Ce code a été fait à mon avis par quelqu'un d'expérimenté, donc je ne critique pas à priori.

Je n'en suis pas si sûr. D'abord la présence d'une variable nommée "uneVariable" me fait lever les yeux au ciel et ensuite il vérifie la valeur de isSet alors que celle-ci ne peut être que fausse (si elle ne l'était pas à l'entrée de la fonction elle l'est dorénavant suite au memset). Enfin il assigne une nouvelle valeur au pointeur uneVariable sans détruire l'ancienne valeur qu'il venait juste d'instancier, ce qui crée une fuite mémoire.

Maintenant, concernant l'appel explicite au destructeur, pourquoi pas s'il s'agit de nettoyer la classe afin de la recycler. L'appel à memset ne fait que réinitialiser les champs que le destructeur n'avait aucune raison de nettoyer.

EDIT: boulette

[deubelte]

Je n'en suis pas si sûr. D'abord la présence d'une variable nommée "uneVariable"

j'ai oublié de précisé que c'est moi qui a modifié les noms. En vrai, ca s'appelle pas comme ca.

[therwald]

Enfin il assigne une nouvelle valeur au pointeur uneVariable sans détruire l'ancienne valeur qu'il venait juste d'instancier, ce qui crée une fuite mémoire.

Comme il déréférence, il me semble que c'est plutôt une assignation. Un appel explicite au destructeur + memset semble plutôt tordu, de fait. Après tout pourquoi ne pas avoir une fonction de nettoyage, qu'on peut appeler du destructeur et de ce point de code?

[deubelte]

Comme il déréférence, il me semble que c'est plutôt une assignation.

Exactement. Il alloue une plage mémoire dont l'adresse est récupérée dans uneVariable, puis il fait appel à l'operateu =.

Un appel explicite au destructeur + memset semble plutôt tordu, de fait.

Pq?

Après tout pourquoi ne pas avoir une fonction de nettoyage, qu'on peut appeler du destructeur et de ce point de code?

Je sais pas, c'est pas moi qui a fait ce code.

[DonQuiche]

Comme il déréférence, il me semble que c'est plutôt une assignation.

Oups, oui, en effet, au temps pour moi.

[therwald]

Un appel explicite au destructeur + memset semble plutôt tordu, de fait.

Pq?

Parce que dans l'utilisation, normale, un appel destructeur se fait implicitement à la suite de la fin de vie de l'objet (sortie de scope d'un objet local, appel à l'opérateur delete). On peut torturer le système mais comme le montre ton étonnement ce n'est pas une manière très claire de coder...

Je sais pas, c'est pas moi qui a fait ce code.

Ma question était réthorique, c'était une manière de présenter ce qui me semblerait un codage plus propre de cette réinitialisation de l'objet.

[deubelte]

Parce que dans l'utilisation, normale, un appel destructeur se fait implicitement à la suite de la fin de vie de l'objet (sortie de scope d'un objet local, appel à l'opérateur delete). On peut torturer le système mais comme le montre ton étonnement ce n'est pas une manière très claire de coder...

Indépendamment de savoir si c'est bien ou pas bien, je voudrais savoir ce qui a amené le developpeur à faire ceci. En gros, que fait la fonction UneFonction. Pour moi, elle consiste à virer l'information qui se trouve dans la plage mémoire pointée par this, puis en renseigner dans la même plage mémoire l'information contenue dans IT_member.

[therwald]

Après en l'absence du reste du code on ne peut pas t'en dire beaucoup plus. Ceci dit, le memset fait ça de façon brutale, et ne permet pas de documenter le pourquoi du comment.
On pouvait faire autrement. Si ce hack résulte d'une réflexion dans un contexte un peu exotique, il aurait dû être accompagné d'un commentaire permettant sa compréhension. Je t'encourage d'ailleurs à ajouter ledit commentaire si tu parviens à te faire une idée claire des tenants et aboutissants de ce code.

[DonQuiche]

Après en l'absence du reste du code on ne peut pas t'en dire beaucoup plus. Ceci dit, le memset fait ça de façon brutale, et ne permet pas de documenter le pourquoi du comment.

Cela ne me semble tout de même pas incompréhensible : à vue de nez le destructeur se contente de nettoyer les objets alloués tandis que le memset vient remettre derrière les états à zéro, une opération qu'on ne fait pas dans un destructeur. Ça peut heurter la première seconde mais ça reste assez intuitif.

Maintenant, oui, une fonction destroyAllocatedObjects utilisée communément par le destructeur et cette fonction aurait été plus explicite et plus "propre". Mais bon... Ça ne me choque pas particulièrement.

[koala01]

Salut,

Malgré le fait qu'il manque quelques informations (pourrais tu, par exemple nous dire s'il n'y a pas un operator new défini pour la classe ) j'aurais tendance à penser au placement new

Il faut savoir que new fait en réalité deux choses bien disticntes:

1. Allouer un espace mémoire suffisant pour permettre de représenter l'ensemble de la structure (l'équivalent à un malloc, en sommes )
2. appeler le constructeur (qui initialise les membres de la classe/ structure)

De plus, il est possible de définir l'opérateur new pour les classes, au même titre qu'il est possible de définir les opérateurs =, == ou autres, ce qui aura pour effet d'appeler cette définition particulière de new quand une allocation dynamique est effectuée

Entre parenthèses , n'oublies pas que l'allocation dynamique de la mémoire est un processus particulièrement lent!!!

Pour certains types d'objet, il est possible (et parfois décidé) de "pré allouer" un espace de mémoire permettant de représenter directement plusieurs objets de manière contigüe.

Il ne reste alors "plus qu'à" initialiser les différents objets "en cas de besoin", et à veiller à ce qu'ils soient correctement détruits (comprenez : que leurs membres soient correctement détruits et placés dans un état "neutre"), sans libérer la mémoire correspondante une fois que l'objet n'est plus utilisé, de manière à pouvoir réutiliser la mémoire un peu plus tard, le tout en ne payant qu'une fois le "prix" de l'allocation et dynamique de la mémoire et de sa libération

[screetch]

c'est du code relativement pourri puisque déjà il detruit l'objet this au milieu d'une méthode, du coup pendant un laps de temps l'objet this n'est plus valable (ce qui est sans doute un comportement indeterminé)

mais surtout parce que ce gros sagouin appelle un destructeur, puis n'appelle pas de constructeur, donc l'bjet après coup est toujours dans l'état detruit. par exemple, si tu ajoutes une methode virtuelle dans l'objet, elle vient d'être foutue en l'air dans cette methode.

c'est pourri et il y a moyen de faire la meme chose proprement, c'est presque inexcusable.

[deubelte]

Voici la struc avec plus d'élément (enfin, une partie de la struct)
Vous voyez que la struct est un encapsulateur d'une union.
De ce fait, l'objet this ne peut être qu'un seul élément de l'union. C'est moi qui a modifié les noms des variables.

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
struct IT_DECLSPEC_vot VSIVOT{
private:
 
    unsigned char isSet;
 
    VSIVOTKind __d;
    union  {
        AAAA* _aaaa_;
        BBBB* _bbbb_;
        CCCC* _cccc_;
        DDDD* _dddd_;
        EEEE* _eeee_;
        FFFF* _ffff_;
        GGGG* _gggg_;
        HHHH* _hhhh_;
        IIII* _iiii_;
        JJJJ* _jjjj_;
	KKKK* _kkkk_;
    };
 
public:
 
    VSIVOTKind _d () const;
    void _d (VSIVOTKind);

Deux geters:

Code :

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

1
2
3
4
5
6
7
8
9
 
public:
    AAAA& FluxBasic () {
        return (*_aaaa_);
    }
 
    const AAAA& FluxBasic () const {
        return (*_aaaa_);
    }

Ici, il s'agit d'une sorte de "swaper". Cela détruit l'ancien objet pointé par this, et réalloue avec un nouvel objet.

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
    void FluxBasic (const AAAA& IT_member) {
        if (isSet && (__d != ::VSIVOTKind_raw))
        {
            this-> uneClasse::~uneClasse();
            memset(this, 0, sizeof(*this));
        }
 
        __d = ::VSIVOTKind_raw;
 
        if ( !isSet )
        {
            _aaaa_= new AAAA;
            isSet = 1;
        }
 
        *(_aaaa_) = IT_member;
    }

Y a quand même la présence d'une UNION, ce qui est assez rare en C++ d'après ce que j'ai vu

[screetch]

ce que le destructeur fait, ca pourrait être fait dans une méthode destroy et personne ne râlerait (avec le memset) (voir même avec le isset)

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
void destroy()
{
    if(isSet)
    {
            .... //ce que le destructeur fait
            memset(this, 0, sizeof(*this));
    }
}
 
    void FluxBasic (const AAAA& IT_member) {
        if (!isSet || __d != ::VSIVOTKind_raw)
        {
            // crée un nouveau AAAA, puisqu'on a soit le mauvais type
            // soit pas de type
            destroy();
            _aaaa_= new AAAA;
            isSet = 1;
        }
         *(_aaaa_) = IT_member;
    }

voila ca fait la même chose, moins de ligne de code et pas d'appel a un destructeur comme un cochon.

[JolyLoic]

c'est du code relativement pourri puisque déjà il detruit l'objet this au milieu d'une méthode, du coup pendant un laps de temps l'objet this n'est plus valable (ce qui est sans doute un comportement indeterminé)

Le comportement est déterminé à condition qu'après delete this, tu n'accèdes plus à aucune donnée membre ni fonction virtuelle de ta classe.

Le cas typique est une classe qui se désenregistre auprès de se conteneur, puis se suicide.

Par contre, là, je n'ai pas tout compris ce qui se passe, ne sachant pas entre autre ce que vaut _uneVariable_. J'aurais tendance à être très méfiant. Peut-être que ça peut marcher si la classe considérée est un POD (mais dans ce cas là, le delete this me semble assez inutile).

[screetch]

Oui, ici l'objectif est d'appeler le destructeur et puis de continuer a travailler avec this. C'est donc indeterminé dans un sens (encore qu'on sait tres bien ce qui se passe)

[deubelte]

Par contre, là, je n'ai pas tout compris ce qui se passe, ne sachant pas entre autre ce que vaut _uneVariable_. J'aurais tendance à être très méfiant. Peut-être que ça peut marcher si la classe considérée est un POD (mais dans ce cas là, le delete this me semble assez inutile).

Dans mon cas,_uneVariable_ est une instance d'une struct qui contient
des string, des Long, des doubles, ainsi que les fonctions :

Code :

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

1
2
3
4
  MaClasse(const MaClasse&);
    MaClasse();
    ~MaClasse();
    MaClasse& operator= (const MaClasse&);