C'est exprès que tu définis une conversion implicite?
Version imprimable
Non plus. J'ai tapé le truc à la va-vite car je voulais surtout mettre en évidence l'idiome copy-swap (ce à quoi j'ai magnifiquement échoué parce que j'ai mis un const de trop :oops:)...
J'ai corrigé, et je viens d'en profiter pour rajouter un constructeur par défaut.
re-re-re-corrigé!
Dis, quand on aura fini, on pourra supprimer les postes de corrections ? Ils me dépriment un peu...
(d'un autre côté, je n'ai jamais voulu cacher que c'était codé sur le pouce)
- L'habitude de la différence entre () et (void) en C, je préfère mettre systématiquement (void).
- Et donc, ça, ça marche, ça compilera et ça fera exactement ce qu'on veut que ça fasse ? (hormis le contrôle sur les exceptions)
Code:
1
2
3 explicit CSmartPointer(T *p) : p(p), pr(new int(1)) { }
Je ne sais pas pourquoi mais j'avais en tête que ce n'étais pas possible, j'ai du le vérifier pour le croire :D, mais au final je suppose qu'il est quand même plus clair d'avoir deux noms différents et ça ne coute pas bien cher non... (enfin je suppose que c'est une question de choix personel)...?Citation:
Envoyé par corrector
Je ne suis pas sûr de tout saisir (:oops:), quelle serait le code adapté alors... :Citation:
Envoyé par corrector
Dans ce cas précis, la gestion d'un bloc catch_try ne coûte pas trop cher en terme de performance?Code:
1
2
3
4
5
6
7
8
9
10 CSmartPointer(T *_p) : p(_p) { try{ this->pr = new int(1); } catch(bad_alloc &) { delete p; throw; } }
Juste par curiosité, dans quelle cas ce genre d'exception arrive en général (je n'ai jamais été confronté à ce problème :oops:)?
Et tant qu'à faire, n'est-il pas préférable d'éviter les exceptions justement :Code:
1
2
3 CSmartPointer(T *_p) : p(_p), pr(new(nothrow) int(1)) { }
Dans un constructeur, pas trop.
Je retire ce que j'ai dit sur le nothrow (:oops:) la suite de mon code est basé sur un compteur sencé être alloué correctement donc... voila le code complet de ma classe :Et l'implémentation :Code:
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 template <class T> class CSmartPointer { public: CSmartPointer(T *Pointer = NULL); CSmartPointer(CSmartPointer<T> const &SmartPointer); ~CSmartPointer(); CSmartPointer<T> & operator = (CSmartPointer<T> const &SmartPointer); CSmartPointer<T> & operator = (T *Pointer); bool operator () (); T const * const operator -> () const; T * const operator -> (); T const & operator * () const; T & operator * (); operator T const * const() const; operator T * const(); void Swap(CSmartPointer<T> &SmartPointer); template <class Other> operator CSmartPointer<Other>() const; private: T *m_Pointer; // Pointer on the object int *m_Counter; // Counter on the object pointed };
Je ne suis pas encore bien habitué au mot clé explicit mais dans mon projet j'utilise :Code:
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
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106 template <class T> inline CSmartPointer<T>::CSmartPointer(T *Pointer) : m_Pointer(Pointer), m_Counter(NULL) { try{ m_Counter = new int(1); } catch(std::bad_alloc &) { SAFE_DELETE(m_Pointer); throw; } } template <class T> inline CSmartPointer<T>::CSmartPointer(CSmartPointer<T> const &SmartPointer) : m_Pointer(SmartPointer.m_Pointer), m_Counter(SmartPointer.m_Counter) { ++(*m_Counter); } template <class T> inline CSmartPointer<T>::~CSmartPointer() { if( --(*m_Counter) == 0 ) { delete m_Pointer; delete m_Counter; } } template <class T> inline CSmartPointer<T> & CSmartPointer<T>::operator = (CSmartPointer<T> const &SmartPointer) { CSmartPointer<T> CopyTmp(SmartPointer); Swap(CopyTmp); return *this; } template <class T> inline CSmartPointer<T> & CSmartPointer<T>::operator = (T *Pointer) { CSmartPointer<T> CopyTmp(Pointer); Swap(CopyTmp); return *this; } template <class T> inline bool CSmartPointer<T>::operator () () { return(m_Pointer!=NULL); } template <class T> inline T const * const CSmartPointer<T>::operator -> () const { Assert( m_Pointer != NULL ); return m_Pointer; } template <class T> inline T * const CSmartPointer<T>::operator -> () { Assert( m_Pointer != NULL ); return m_Pointer; } template <class T> inline T const & CSmartPointer<T>::operator * () const { Assert( m_Pointer != NULL ); return (*m_Pointer); } template <class T> inline T & CSmartPointer<T>::operator * () { Assert( m_Pointer != NULL ); return (*m_Pointer); } template <class T> inline CSmartPointer<T>::operator T const * const() const { return m_Pointer; } template <class T> inline CSmartPointer<T>::operator T * const() { return m_Pointer; } template <class T> inline void CSmartPointer<T>::Swap(CSmartPointer<T> &SmartPointer) { std::swap(m_Pointer, SmartPointer.m_Pointer); std::swap(m_Counter, SmartPointer.m_Counter); } template <class T> template <class Other> inline CSmartPointer<T>::operator CSmartPointer<Other>() const { Other *Pointer = (Other *)m_Pointer; return CSmartPointer<Other>(Pointer); }
Avec AddCamera prenant un smart pointer (de CCamera) en paramètre, dans ce cas il faut bien utiliser le constructeur en tant que conversion implicite non?Code:SceneManager.AddCamera( new CCamera(*this) );
Enfin si vous avez d'autres conseils ou critiques sur le code je suis preneur encore merci à tous :D
NOTE : l'opérateur de conversion n'a pas encore était testé(:oops:), je ne suis pas sur que ce soit très correct (enfin j'y viendrai...)
Je pense que tu peux utiliser le swap aussi sur operator= (T*).
Et aussi, je te pose la question avant corrector: Constructeur implicite, c'est voulu ?
En effet ça semble fonctionner, c'est modifé merci ;)Citation:
Envoyé par Médinoc
En prévision a cette question qui m'a était posé 3fois :mouarf: j'ai laissé ce message :Citation:
Et aussi, je te pose la question avant corrector: Constructeur implicite, c'est voulu ?
Pourquoi cette question revient-elle? Il y a des risques (dans mon cas)?Citation:
Envoyé par babar63
OK. En effet, là, le constructeur implicite est requis, je pense.
Et pour les risques, je ne sais pas trop à part des risques de confusion. J'ai surtout posé la question par mimétisme, parce que corrector la posait...
Dans la classe elle-même, tu peux écrire CSmartPointer à la place de CSmartPointer<T>.
Pourquoi faire un foncteur?
Pourquoi la distinction const/non const?
(Soit rigoureux dans ton explication.)Pourquoi faire une fonction séparée? Tu ne l'appelles qu'une seule fois.
Ces parenthèses sont redondantes.
Ces parenthèses sont redondantes.
Ces parenthèses sont redondantes.
Pourquoi SAFE_DELETE?
En effet, ce n'est pas forcément approprié. Mais ça peut aussi l'être, dépendant des circonstances. Enfin, je crois. Mais en y réfléchissant plus, je ne vois plus trop où, les exemples que j'avaient en tête s'écroulent dès que j'y pense plus d'une seconde...Citation:
Pourquoi la distinction const/non const?
Pour bien faire, il faudrait peut-être deux versions de la classe: Une qui fait des contrôles comme ça, une autre où la constance de l'objet est complètement dissociée de celle du pointeur intelligent...
D'après la FAQ c'est un prédicat si j'ai bien compris, je l'utilise simplement pour tester si mon objet pointé est valide(NULL) ou non.Citation:
Envoyé par corrector
N'est-il pas correct d'utiliser un maximum de const lorsqu'on le peut? Voila un exemple de mon projet qui utilise la fonctionCitation:
Envoyé par corrector
Code:T const * const operator -> () const;
Je suppose que je n'ai pas besoin de montrer d'exemple pour le même opérateur sans tous ces consts :), ce n'est donc pas correct? pourquoi?Code:
1
2
3
4
5 //fonction membre de CSeneManager : std::vector<ILightPtr> const & GetLights() const { return m_Scene->Lights; // Un vecteur de ILightPtr membre de la class }
Je fais de mon mieux mais je suis en train d'apprendre donc je ne peux pas toujours expliquer comme il le faudrait, sans doute je l'ai vu quelques part, ça m'a parut intelligent, j'ai gardé l'idée et je la ressort de temps en temps :oops:.Citation:
Envoyé par corrector
Justement je l'appelais dans mon opérateur :Citation:
Envoyé par corrector
Que j'ai modifié à la suite de la remarque de médinoc, mais j'ai oublié de supprimer la fonction après...(je le fais de suite)Code:CSmartPointer<T> & operator = (T *Pointer);
Je préfère parfois mettre un peu plus de parenthèse, dans la limite du raisonnable je trouve le code plus clair... Est-ce un mauvais réflexe?Citation:
Envoyé par corrector
Euh....:roll:.... Pour le coup j'avoue que je ne vois plus donc sans doute une mauvaise raison :oops:. En revanche le deuxième :Citation:
Envoyé par corrector
est correct non? Je l'utilise pour supprimer m_Pointer si il est NULL et le réinitialiser à NULLCode:
1
2
3
4 catch(std::bad_alloc &) { SAFE_DELETE(m_Pointer); throw; }
Le problème, c'est que ce code suppose que dès que le pointeur est constant, les données pointées le sont aussi.Citation:
N'est-il pas correct d'utiliser un maximum de const lorsqu'on le peut? Voila un exemple de mon projet qui utilise la fonction
<snip>
Je suppose que je n'ai pas besoin de montrer d'exemple pour le même opérateur sans tous ces consts , ce n'est donc pas correct? pourquoi?
Si tu veux un smart pointer vers des données constantes, tu peux en faire un qui retourne toujours un T* et juste faire un CSmartPointer< const int > par exemple...
En effet je n'y ai pas pensé... Par exemple pour l'opérateur -> j'ai donc une seule fonction :Citation:
Envoyé par Médinoc
Les premiers tests ont l'air de marcher, c'est correct?Code:T * const operator -> () const;
C'est simple (oublie les considérations théoriques sur l'exception safety); le code suivant ne doit pas causer de fuite de mémoire, quoi qu'il advienne :
Oui, mais pourquoi ne pas trouver attraper toutes les exceptions avec catch(...)?Code:CSmartPointer<T> p (new T);
Tu peux aussi utiliser une "function-try-block" :
La function-try-block d'un constructeur englobe l'initialisation des membres, il correspond à une portée extérieure. Il ne faut donc surtout pas se référer aux membres de l'objet dans le bloc catch :Code:
1
2
3
4
5
6 CSmartPointer(T *p_) try : p(p_), pr(new int(1)) { } catch(...) { delete p_; }
Comme c'est un constructeur, atteindre la fin du bloc catch (le "}" fermant) relance l'exception (tout comme "throw;").
- certains n'ont pas été construits parce qu'une exception a interrompu l'exécution de la liste d'initialisation,
- les autres ont été détruits avant d'entrer dans le bloc catch
Sur une implémentation des exceptions à base de tables, le cout en temps d'exécution est nul. La taille de l'exécutable va très légèrement augmenter.
Euh... sans doute parce que je ne connaissait pas tout ça :D, en tout cas merci c'est toujours bon à savoir.Citation:
Envoyé par corrector
Si si, je viens de me relire c'était le sens de ma question, mais ce n'était pas voulu (:oops:) je voulais dire : "Je l'utilise pour supprimer m_Pointer si il est différent de NULL et le réinitialiser à NULL". Et puis au risque de ne toujours pas être clair voila la définition :Citation:
Ce code est parfaitement valide (et inutile, évidemment).
(Mais je ne suis pas sûr si c'était le sens de ta question.)
NOTE : mon SAFE_DELETE ne serait pas très "safe" si il ne désallouait que les pointeurs NULL :mouarf:Code:#define SAFE_DELETE(x) if(x) {delete x; x=0;}
NOTE2 : Encore une fois je tiens à remercier tout le monde pour les réponses apportées, ainsi que de m'avoir aidé à mieux comprendre tout ça! (surtout que je suis parfois à peine têtu) :merci:
Encore une erreur, je vais battre le record en un topic :roll:. Je suppose qu'UNE PARTIE du problème peut se résoudre comme ça :En revanche il reste toujours un problème mais je ne vois pas comment le régler :Code:#define SAFE_DELETE(x) { if(x) {delete x; x=0;} }
Code:
1
2 if (cond) SAFE_DELETE(p); // Compile
Est-ce vraiment nécessaire de pouvoir compiler ce code sachant que :Code:
1
2
3
4 if (cond) SAFE_DELETE(p); else // Erreur : "instruction else sans if correspondant non conforme" ...
Et puis sachant aussi que le ";" est inutile?Code:
1
2
3
4
5
6 if (cond) { SAFE_DELETE(p); } else // Ok ...
EDIT : J'ai oublié, éventuellement une fonction inlinée pourrais remplacé le define, mais est-ce vraiment utile... ?
C'est là où je voulais en venir : pourquoi s'embêter avec les macros et les problèmes associés, dont tu as eu un aperçu, alors que tu peux écrire une vraie fonction normale, inline pour éviter tout surcout. (Mais, par rapport à delete, un appel de fonction n'est pas bien lourd.)
Dans ce cas précis, quelles sont les problèmes associés que l'ont n'aurait pas avec une fonction, exemple :Citation:
Envoyé par corrector
et :Code:
1
2
3
4
5 if (cond) SAFE_DELETE(p); else // Macro : Erreur // Fonction : Ok ...
Mis à part le fait que les macros "semblent" être de moins en mois appréciées quand elle peuvent être évitées (enfin je dis ça d'après ce que j'en ai lu..), dans ce cas c'est bien une question de choix non?Code:
1
2
3
4
5 if (cond) SAFE_DELETE(p) else // Macro : Ok // Fonction : Erreur ...
Ce que j'ai montré précédemment, déjà. (Il y en d'autres, essaie de les trouver.)
Ce n'est pas que les macros sont de moins en moins appréciées, elles sont détestées (en plus, le préprocesseur est un très mauvais langage de macro : peu puissant, difficile à définir et à implémenter, en plus il est mal intégré au langage).
On les utilise quand on n'a pas mieux.
Quand une fonction fait l'affaire, utiliser une macro est considéré comme une erreur grossière.
Oui, c'est bien l'usage d'un constructeur de conversion ("converting constructor") :
Comme il n'y a pas de mot-clef "implicit", l'absence d'explicit sur un constructeur à un argument peut être un simple oublie, sans un commentaire comment savoir.Code:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17 class A { public: explicit A (int); A (const char*); // non-explicit }; void foo (A); int main() { foo (1); // erreur foo ("1"); // OK, équivaut à : foo (A("1")); A a = 2; // erreur A a = "2"; // OK, équivaut à : A a = A("2"); }
Le risque de la conversion implicite, c'est qu'elle est implicite (n'est-ce pas), alors tu peux l'invoquer par erreur, et, par exemple, donner à un smart pointer la propriété d'un pointeur, sans le vouloir.
La crainte avec les conversions implicites c'est de les faire sans le vouloir, et qu'un code incorrect compile grâce à elles. En pratique, il faut analyser l'opportunité de déclarer des conversions implicites au cas par cas.
Ouha... quelle influence j'ai!
Essaie:
Qui devrait résoudre les problèmes de 'if' imbriqués et le problème du ';'.Code:
1
2#define SAFE_DELETE(p) do { if(x) { delete (x); (x)=0; } } while (false)
Ceci dit, la meilleure manière de le faire reste:
Autre chose, cette fonction n'est pas un safe_delete mais un delete_to_null. Rien à voir (au niveau du nom et de la fonction réalisée).Code:
1
2
3
4
5
6
7 namespace { template <class T> void safe_delete(T*& p) { if (p) { delete p; p = NULL; } } }
Merci pour ces précisions, comme conseillé par corrector, j'ai déjà créé une fonction :J'aurais quand même une ou deux questions par rapport à ton code, pourquoi utiliser un namespace sans nom? Cela change-t-il quelques choses? Aussi je ne vois pas la différence entre "T *&" et "T *"... :aie: ?Code:
1
2
3
4
5
6
7
8
9 template <class T> inline void SafeDelete(T *ptr) { if(ptr!=0) { delete ptr; ptr=0; } }
C'est simple: Sans le &, ton ptr=0 ne sert à rien.Citation:
Aussi je ne vois pas la différence entre "T *&" et "T *"... ?
En effet ça a du sens, merci pour la précision :DDonc si j'ai bien compris, dans mon cas c'est à éviter, puisque je souhaite pouvoir réutiliser cette fonction dans d'autre projets(/fichiers).Et je vais aussi la renommer :mouarf:, à la base j'ai donné ce nom car j'ai vu cette fonction(Macro) plusieurs fois sous ce nom (:oops:) mais en effet il ne semble pas y avoir de bonnes raisons (enfin à ma connaissance)...Citation:
Autre chose, cette fonction n'est pas un safe_delete mais un delete_to_null. Rien à voir (au niveau du nom et de la fonction réalisée).
Encore merci à tous pour tous ces précieux conseils :D