Discussion :

[zoya23]

Bonjour,

Mon programme plante lorsque j'utilise directement le membre list "facts" pour le remplir via la fonction setFactsList2.

Voici le code :

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
25
26
27
28
29
30
31
32

Entity.h :

class Entity
{
    public :
        int id;
        string name;
        list <Fact> facts;  //facts
        
    public :
        ... 
        void  setFactsList2(const std::list<Fact>& newFacts);
}

Entity.cc :

void  Entity::setFactsList2(const std::list<Fact>& newFacts) {
    for (std::list<Fact>::const_iterator i = newFacts.begin(); i != newFacts.end(); ++i) {
        facts.push_back(*(i));
    }
}

void Entity::addFacts(list<Fact> factsSource, Entity *entity){

    list<Fact> factsDest = entity->getFactsList();

    unit(factsSource, &factsDest);

    entity->setFactsList2(factsDest);
}

Lorsque je commente la ligne "facts.push_back(*(i));", l'execution fonctionne.

Auriez vous une idée sur l'origine du problème ?

Merci d'avance.

[ternel]

Quand tu dis que ca plante, ton pc ralenti, puis freeze?
Est-ce que par hasard, tu ne serais pas en train d'ajouter une liste à elle-même?

Au quel cas, c'est une boucle infinie, qui échoue par plantage mémoire.

Tu n'oserai pas, par le jeu des références, faire e.setFactsList2(e.list);?


Cela dit, d'un point de vue conceptuel, setFactList (qui devrait s'appeler setFacts) devrait commencer par list.clear();, ou s'appeler addFacts();

[KsassPeuk]

Lu'!

Déjà une petite remarque :

Code :

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

1
2
3
for (std::list<Fact>::const_iterator i = newFacts.begin(); i != newFacts.end(); ++i) {
   facts.push_back(*(i));
}

Plus simplement :

Code :

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

facts.insert(facts.end(), newFacts.begin(), newFacts.end());

Sinon, que dis le debugger a propos de ce plantage ?

Dans ton code, tu accès à un pointeur sur une entité, cependant, tu ne vérifies pas (par assert) que ce pointeur est valide. D'ailleurs pourquoi autant de pointeurs ?

[zoya23]

Merciiii beaucoup pour votre réponse,

En fait, initialement mon entité contient des faits "facts" et je veux rajouter d'autres faits "factsSource".

J'ai allégé mon code comme suit :

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

Entity.cc :

void Entity::addFacts(list<Fact> factsSource){
    
    std::list<Fact>::iterator iterator;
    for (iterator = factsSource.begin(); iterator != factsSource.end(); ++iterator) {                
                this->facts.push_back((*iterator));
            }
}

Main class :

//appel à la fonction addFacts
std::list<Entity>::iterator iterator3;
for (iterator3 = targetPeers.begin(); iterator3 != targetPeers.end(); ++iterator3) {                    
       (*iterator3).addFacts(P0.facts);
}

Avec ce code, j'ai toujours le même problème. Quand j'utilise directement le membre "facts", ça plante.
Le message "MonProjet.exe a cessé de fonctionner".

[KsassPeuk]

Avec ce code, j'ai toujours le même problème. Quand j'utilise directement le membre "facts", ça plante.
Le message "MonProjet.exe a cessé de fonctionner".

Sinon, que dis le debugger a propos de ce plantage ?

Ta boucle dans la fonction d'ajout est toujours inutile (voir mon précédent post), de même que le this->.

Également :

Code :

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

1
2
3
4
std::list<Entity>::iterator iterator3;
for (iterator3 = targetPeers.begin(); iterator3 != targetPeers.end(); ++iterator3) {                    
   (*iterator3).addFacts(P0.facts);
}

Ou plus simplement :

Code :

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

1
2
for(auto& t : targetPeers)
  t.addFacts(P0.facts);

Tu as ajouté des contrôles sur tes pointeurs ?

[PilloBuenaGente]

Tu as ajouté des contrôles sur tes pointeurs ?

Et je dirais même plus (si je peux me permettre).

Code :

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

void Entity::addFacts(list<Fact> factsSource, Entity *entity)

Evitez au maximum les pointeurs nus, ceci devrait resembler à cela:

Code :

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

void Entity::addFacts(std::list<Fact> factsSource, std::unique_ptr<Entity>& entity)

Aussi je ne vois pas pourquoi tantôt il y a std:: et tantôt rien. Peut être du au recopiage, sinon enlevez le using namespace std, en tout cas dans le header.

[JolyLoic]

Evitez au maximum les pointeurs nus, ceci devrait resembler à cela:

Code :

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

void Entity::addFacts(std::list<Fact> factsSource, std::unique_ptr<Entity>& entity)

Pas d'accord : Ce qu'il faut éviter, ce sont les pointeurs nus possédant. Les pointeurs nus observants restent tout à fait légitimes. Et passer une argument par pointeur quand le pointeur peut être nul et que la fonction ne va pas avoir d'ipact sur la durée de vie de l'objet est la manière recommandée de faire. Ici, un passage par référence constante me semblerait le plus approprié.

[PilloBuenaGente]

Je trouve étrange le fait que l'OP utilise bien les références constantes dans sa fonction setFactsList.

Admettons l'usage des pointeurs est justifié, et qu'ils soient stockés dans un container quelquonque, les pointeurs intelligents seraient "toujours" recommandés ?!
(à moins peut être qu'ils soient placés dans une classe non copiable et non assignable)

Récuperer un élement reviendrait, suivant l'exemple, à avoir :
myEntity.addFacts(factsSource, smartPointerEntityContainer[num]); et non pas
myEntity.addFacts(factsSource, smartPointerEntityContainer[num].get()); ?!

Bref, désolé si mon manque d'experience me fait dire des conneries

[fenkys]

Sans tenir compte de la qualité de ton code, ce que d'autre ont fait auparavant, il y a plusieurs raisons pouvant expliquer le plantage.

On ne connait pas l'object fact.
- contient il un pointeur ?
- si oui, as tu défini le constructeur de copie et l'opérateur d'affectation ?
- ton opérateur d'affectation est il protégé contre l'autoaffectation ?

Comment utilises tu l'objet Entity ?
- par une variable globale ?
- par une variable locale ?
- par un pointeur ?
- si c'est un pointeur, est il correctement alloué ?

Il y a certainement d'autres points à vérifier.

[koala01]

Salut Zoya, et bienvenue sur le forum.

Nous tenons à te remercier d'avoir veillé à fournir un peu de code, mais le meilleur moyen d'obtenir de l'aide, lorsque tu es face à un problème qui n'apparait qu'à l'exécution, c'est d'essayer de nous donner un code minimal reproduisant le problème.

L'idée n'est donc pas de nous fournir le code complet de ton application (qui peut se chiffrer en plusieurs dizaines / centaines / milliers de fichiers), mais d'essayer de supprimer de ton code tout ce qui n'a rien à voir avec le problème de manière à n'avoir qu'un nombre de fichiers / de fonctions le plus petit possible qui permette de reproduire le problème

En effet, une application n'est jamais qu'un ensemble d'appels de fonction qui appellent d'autres fonctions et le résultat dépend très fort de l'ordre dans lequel les différentes fonctions sont appelées.

Mais, en attendant, nous pouvons déjà essayer de te donner quelques pistes à explorer :

1. Respecte la const correctness :
   
   • chaque fois qu'une fonction membre ne doit pas pouvoir modifier l'objet au départ duquel elle est appelée, déclare la constante
   • Chaque fois que tu transmet un paramètre à une fonction qui ne peut pas le modifier, transmet le sous une forme constante
2. Fais attention à la sémantique (d'entité Vs de valeur) de tes classes : si héritage --> sémantique d'entité d'office --> non copiable et non affectable
3. Préfères les références aux pointeurs chaque fois que tu le peux (un pointeur n'est utile que lorsque l'objet à transmettre / utiliser peut ne pas exister)
4. Chaque accès à un pointeur doit être "sécurisé" par un test s'assurant que le pointeur existe bien (!= nullptr)
5. Il est très facile de mettre la mémoire sens dessus dessous avec la gestion dynamique de la mémoire. Utilises les pointeurs intelligents (soit issu de la SL si tu dispose de C++11, soit ceux issus de boost). Cela te donnera certaines garanties propres à t'éviter pas mal d'erreur.
6. Essaye de penser à tes classes en termes de services plutôt qu'en termes de données, les données n'étant là que pour assurer les services offerts...
   
   • Un accesseur (getter) peut s'avérer utile s'il correspond effectivement à un service que l'on est en droit d'attendre de la part de la classe
   • Un mutateur (setter) n'est que très rarement utile : la manière dont les données sont gardées en mémoire au niveau de ta classe doit rester... au niveau de la classe (l'utilisateur de la classe n'a pas à savoir que tu utilises une liste, un tableau ou un arbre binaire perso pour maintenir les données au sein de ta classe
7. J'en oublie surement