initialisation incorrecte dans une boucle for

**miq75** · 09/09/2010, 14h00

Bonjour,

J'ai le code suivant :

Code :

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

1
2
3
4
5
6
7
cout << " 1 -> " << * (nodes().begin()) << endl;
for (deque<Position*>::iterator pit = nodes().begin(); pit != nodes().end(); ++pit)
{
	cout << " 2 -> " << * pit << endl;
	cout << " 3 -> " << * (nodes().begin()) << endl;
	...
}

Ce code, au milieu d'une grosse appli. plante une fois sur deux en moyenne.
voilà le résultat quand le comportement n'est pas celui attendu :

Code :

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

1
2
3
 1 -> 0xb6d04968
 2 -> 0xb6d00040
 3 -> 0xb6d04968

Quand ça ne plante pas, les 3 lignes renvoient bien entendu la même adresse. Je ne comprends pas comment il est possible que ces résultats soient différents.

La fonction nodes() renvoie un deque<Positions*> construit bien avant que ces lignes ne s'exécutent.

Existe il une méthode nodes par défaut dans les objets c++ qui pourrait poser un problème d'accès ?

Mon appli utilise plusieurs thread, mais qui ne partagent jamais les même variables. Le scheduling peut il influencer le résultat de mon code ?

Sinon, d'où vient ce problème ?

**miq75** · 09/09/2010, 15h17

Bon, il semblerait que lorsque je remplace dans ce bout de code l'appel à la méthode nodes() par un accès direct à l'attribut _nodes dans ma classe de base (changé pour l'occasion en portée "public"), cet accès réussisse systématiquement.

Pourtant, la méthode nodes() de la classe de base ne fait qu'une redirection vers l'attribut _nodes de cette même classe de base.

Il semble donc que l'appel à nodes() foire de temps en temps. J'ai changé le nom de la méthode sans que cela ne change le résultat, il n'y a donc pas écrasement du nom par un autre.

Mais comment expliquer que, parfois, cet accès soit foiré quand on passe par la méthode ? Serait ce une défaillance de l'allocation mémoire ? Y as-t'il autre chose qu'un itérateur sur un pointeur qui soit alloué dans l'initialisation de ma boucle for ?

Comment faire un code propre (en évitant de mettre les attributs de la classe de base en public) sans que ce bout de code ne plante ?

**Aleph69** · 09/09/2010, 16h00

Bonjour,

tu peux donner ta fonction nodes() pour voir?
Peut-être qu'elle renvoie une copie de ton deque?

**miq75** · 09/09/2010, 16h16

La voilà, dans le fichier header:

Code :

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

1
2
3
4
5
6
7
8
9
10
 
template <class N_, class E_> class Graph
{
...
public:
    std::deque<N_*> nodes() {return _nodes;};
...
private:
    std::deque<N_*> _nodes;
}

Ça n'expliquerai pas pourquoi des fois ça marche, et des fois non... La deque en l'occurrence n'est jamais vide. Si l'initialisation du for ne plante pas au premier passage, elle ne plante plus dans l'exécution, alors qu'elle repasse des dizaines de fois dedans.

**Aleph69** · 09/09/2010, 16h45

Bon, je ne connais pas bien deque donc ça ne va peut-être pas t'aider mais, au cas où, voici quelques pistes.

Dans la référence, je note deux choses :

le constructeur par copie fait une initialisation par défaut :
Code : Sélectionner tout - Visualiser dans une fenêtre à part
```
1
2
 
explicit deque ( const Allocator& = Allocator() );
```
Du coup, je ne sais pas ce qui se passe si _nodes n'est pas alloué...

la fonction begin() est implémentée avec des constantes :

Code :

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

1
2
 
const_iterator begin () const;

Je me demande si tu ne devrais pas t'en inspirer pour ta fonction nodes() :

Code :

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

1
2
 
std::deque<N_*> const nodes() const{return _nodes;};

De manière générale, c'est mieux d'utiliser des constantes pour les accesseurs.

**Aleph69** · 09/09/2010, 21h16

Après réflexion, si _nodes n'était pas alloué, ça lancerait une exception probablement.

**miq75** · 09/09/2010, 22h40

Bon, j'ai testé en modifiant la chaine de création/utilisation de l'itérateur avec des consts, ça ne change rien.

_node est bien alloué en amont dans le programme.

Parfois, quand je passe dans le for, la copie du pointeur plante et il me renvoie une adresse erronée.

**Mat007** · 10/09/2010, 18h28

J'ai oublié de dire que pour corriger ton problème il te suffit de renvoyer des références :

Code :

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

1
2
3
4
5
6
7
8
9
10
 
template <class N_, class E_> class Graph
{
...
public:
    std::deque<N_*>& nodes() {return _nodes;};
...
private:
    std::deque<N_*> _nodes;
}

Mais oui comme le souligne koala, dans l'absolu ça serait encore mieux si c'était encapsulé...

MAT.

**miq75** · 13/09/2010, 08h54

Bonjour,

Aleph, non, je ne modifie par mon conteneur durant l'itération
et la seule modification de l'itérateur est le ++ contenu dans la boucle.

Tu as tout a fait raison koala, d'autant plus que j'ai déjà fourni les méthodes pour accéder aux itérateurs directement au lieu d'accéder au conteneur de la classe. C'est d'ailleurs en les utilisant directement que je vais corriger ce bug dans mon prog, merci de me les avoir rappelé

Mat, effectivement, ça doit être ça la raison. L'execution du for étant un poil plus lente que celle de la ligne directe, il doit avoir le temps de purger la mémoire de la copie lors de certaines itérations. Perso, je trouve ça bizarre comme comportement, mais bon, c'est dans le pur style c++ :/

Merci les gars

**Aleph69** · 09/09/2010, 23h35

Bonsoir,

avec google, j'ai trouvé un avertissement concernant les itérateurs :

Iterators to exisiting elements in a container can become invalidated when the container is changed. This makes changing the container while iterating it problematic.

Tu ne modifies pas ton conteneur pendant que tu itères?

**Aleph69** · 09/09/2010, 23h43

Autre question : est-ce que tu modifies l'itérateur pit dans ta boucle?

**koala01** · 10/09/2010, 04h00

Salut,

Juste une petite remarque au passage: tu ne devrais pas permettre à l'utilisateur d'accéder à une collection d'objet qui est membre de ta classe.

Que ta collection d'objets (ici des pointeurs sur N) soit une deque, une liste, un vecteur ou n'importe quoi, cela fait partie des "détails d'implémentations" de ta classe.

Par contre, ce qui doit intéresser l'utilisateur, c'est:

de pouvoir ajouter un élément
de pouvoir (éventuellement) retirer un élément
de pouvoir obtenir le premier élément
(pour pouvoir "boucler" sur les élément) de pouvoir obtenir "ce qui suit le dernier" élément
plus quelques fonctions "qui vont bien"

Et, pour que ta collections de pointeurs soit "tout à fait transparente à l'utilisateur", tu peux même définir un typedef sur les types d'itérateurs que tu lui permet d'utiliser (surement le const_iterator, peut-être l'iterator "simple").

Au final, ta classe ressemblerait plutôt à quelque chose comme:

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
template <class N_, class E_> class Graph
{
...
public:
    /* Si plus tard, tu viens à décider que la std::deque ne convient
     * pas, il suffira de modifier le typedef en conséquence sans
     * que cela n'implique de changer tous les endroits dans ton
     * code où tu les utilises :D
     */
    typedef std::deque<N_*>::const_iterator const_iterator;
    typedef std::deque<N_*>::iterator iterator;
    const_iterator begin() const{return _node.begin();}
    const_iterator end() const{return _nodes.end();}
    /* les fonction qui vont bien (choisir celles qui t'intéressent :D*/
    bool isEmpty() const{return _nodes.empty();}
    void push_front(T* t){_nodes.push_front(t);}
    void push_back(T* t){_nodes.push_back(t);}
    size_t size() const{return _nodes.size();}
    /* ... */
private:
    std::deque<N_*> _nodes;
};

Il faut te dire que, plus tu en exposera sur ta classe, plus l'utilisateur risque d'être tenté de faire des choses qu'il n'aurait normalement pas du faire.

De plus, il y a la loi demeter qui t'incite réellement à faire en sorte que l'utilisateur de ta classe ne sache "que ce qui l'intéresse", et, le fait que ton graphe soit géré sous la forme d'une std::deque, cela ne l'intéresse absolument pas