Discussion :

[gilloddon]

Bonjour,

Je viens vers vous car j'ai un petit problème de passage de paramètre dans une méthode, et probablement aussi un petit problème de conception.

Pour simplifier, disons j'ai une classe centrale "App" qui a de nombreux traitements à faire et qui est liée à mon Affichage (Ligne de commande, ou Graphique). Elle reçoit donc les évènements des vues.
Etant donné qu'il y a tout un tas de traitements différents, la classe App instancie dans son constructeur plusieurs classes comme par exemple "CloudModule" pour séparer les traitements par "type".
La classe CloudModule a elle-même pas mal de traitements à faire et instancie aussi dans son constructeur plusieurs classes comme par exemple "FileManager".

La classe App est centrale, délègue un traitement à son module, qui déléguera aussi son traitement à un de ses Managers.
Une classe module contient tous les attributs nécessaire pour les traitements (Buffer, entiers, ...).
Les managers ont besoins des attributs du Module pour effectuer leurs traitements.

Voici un petit diagramme de classe simplifié : http://i.imgur.com/ueKTZTV.png



Mon problème se situe entre les "Modules" et les "Managers".
Au début, je me suis dis que je passerais la référence du "Module" à chaque "Manager" en l'instanciant.
Le Manager pourrait ensuite avoir accès aux attributs via la référence du module.

Mais j’ai un problème en faisant ça, à la compilation : error: no matching function for call to 'FileManager::setModule(CloudModule* const)'

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
 
    class CloudModule
    {
        public:
            CloudModule();
            virtual ~CloudModule();
 
        private:
            FileManager fileManager;
    };
 
 
 
    CloudModule::CloudModule()
    {
        this->fileManager = new FileManager();
 
        this->fileManager->setModule(this);
    }

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
 
    class FileManager
    {
        public:
            FileManager();
            virtual ~FileManager();
 
            void setModule(CloudModule *);
 
        protected:
            CloudModule *module;
    };
 
 
 
    FileManager::FileManager()
    {
 
    }
 
    FileManager::~FileManager()
    {
 
    }
 
 
    void FileManager::setModule(CloudModule *aModule)
    {
        this->module = aModule;
    }

J'ai essayé de jouer avec les 'const' dans les paramètres de ma méthode, mais j'arrive pas à passer 'this' non plus si je modifie le prototype de ma méthode.

Code :

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

1
2
3
error: no matching function for call to 'FileManager::setModule(CloudModule* const)'
note: candidate is:
note: void FileManager::setModule(const CloudModule*)

Du coup, je commence aussi à douter, au niveau conception, de la propreté de cette solution.
Est-ce vraiment une bonne façon de répartir des traitements vers plusieurs autres classes ?

Comment est-ce que je pourrais améliorer ce système ?


Merci d'avance.

[foetus]

Réponse vite-faite :

1. Supprime tous les "this->": cela surcharge inutilement
2. Le compilateur te dit ton erreur "FileManager::setModule(CloudModule* const)". Si je ne me trompe pas, tu passes this qui est constant alors que ton prototype lui dit "je peux modifier le paramètre". Donc essaye soit de mettre const dans le prototype si cela est possible soit de caster (le mieux c'est const_cast)

[Ehonn]

setModule est une méthode donc elle connais déjà this.
Tu n'as peut être pas besoin de passer un paramètre ici ?

Dans le code que tu donnes, il y a le constructeur de CloudModule et les déclarations de la classe FileManager.
Du coup, on a pas le prototype de la méthode CloudModule::setModule (qui devrait être selon le code void setModule(CloudModule const *)).

En tout cas, le const_cast est plus qu'à éviter.

[gilloddon]

setModule est une méthode donc elle connais déjà this.
Tu n'as peut être pas besoin de passer un paramètre ici ?

Dans le code que tu donnes, il y a le constructeur de CloudModule et les déclarations de la classe FileManager.
Du coup, on a pas le prototype de la méthode CloudModule::setModule

J'ai modifié mon premier message pour compléter les balises codes, car tu as fais une petite erreur d'interprétation :
La méthode setModule n'est pas dans la classe CloudModule, mais dans la classe FileManager (sinon effectivement, ça ne servirait à rien car CloudModule se connait elle-même ^^).
=> FileManager::setModule(CloudModule *)

qui devrait être selon le code void setModule(CloudModule const *)

C'est la première chose que j'ai essayé quand je me suis aperçu que 'this' était constant, mais ça n'arrange pas les choses.

Code :

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

1
2
3
error: no matching function for call to 'FileManager::setModule(CloudModule* const)'
note: candidate is:
note: void FileManager::setModule(const CloudModule*)

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
class FileManager
    {
        public:
            FileManager();
            virtual ~FileManager();
 
            void setModule(CloudModule const *);
 
        protected:
            CloudModule const *module;
    };
 
 
 
    FileManager::FileManager()
    {
 
    }
 
    FileManager::~FileManager()
    {
 
    }
 
 
    void FileManager::setModule(CloudModule const *aModule)
    {
        this->module = aModule;
    }

Sinon, en dehors de ce problème de 'const', est-ce une bonne idée de passer des références de 'this' dans ce cas là ?
Est-ce qu'il existe des solutions plus efficace/plus propres pour répartir ?

[Iradrille]

(le mieux c'est const_cast)

Pour le reste de ta réponse on est d'accord.
Mais on utilise un const_cast quand une erreur de const-correctness à été commise dans du code que l'on doit utiliser mais que l'on ne peut pas corriger.
(Normalement avec le mot clef mutable, il n'y à pas d'autre utilisation possible de const_cast).

Par exemple une ancienne lib C qui prend un char* au lieu d'un const char* en paramètre. (Ou cas similaire).

[foetus]

Mais on utilise un const_cast quand une erreur de const-correctness à été commise dans du code que l'on doit utiliser mais que l'on ne peut pas corriger.
(Normalement avec le mot clef mutable, il n'y à pas d'autre utilisation possible de const_cast).

Pour moi, le const_cast remplace le cast "à la C" pour un usage spécifique (retirer l'état contant) avec une syntaxe (comme les 2-3 autres cast C++) qui te surcharge ton code afin d'alerter le développeur: "Soit tu mets trop de const, soit tes const sont mal faits, soit il y a un truc qui cloche"

[Bousk]

C'est vraiment là ton seul code et c'est bien CE setModule qui plante ? impossible.
Il n'y a aucune raison de planter, dans le contstructeur, this n'est pas const
D'ailleurs ce code compile parfaitement, moyennant l'ajout du * dans la déclaration du membre fileManager de CloudModule.

[Iradrille]

Pour moi, le const_cast remplace le cast "à la C" pour un usage spécifique (retirer l'état contant) avec une syntaxe (comme les 2-3 autres cast C++) qui te surcharge ton code afin d'alerter le développeur: "Soit tu mets trop de const, soit tes const sont mal faits, soit il y a un truc qui cloche"

Oui un const_cast remplace les cast "style C", tout comme static_cast ou reinterpret_cast (dynamic_cast est différent).

Mais dans les cas d'utilisation que tu donne "Soit tu mets trop de const" est faux, ça devrait plutôt être "Il manque un const quelque part".

[Ehonn]

FileManager::setModule(CloudModule * const) et FileManager::setModule(CloudModule const *) sont différents.
Dans le premier, on a pas le droit de modifier le pointeur, dans le deuxième on a pas le droit de modifier l'objet pointé.
(L'utilisation d'une référence permet de ne pas faire ce genre d'erreur).

[gilloddon]

J'ai mis la mauvaise classe dans la signature de ma méthode, c'était donc normal que le compilateur gueule (quel idée d'avoir deux classes avec un nom quasi-similaire aussi).
Mais quel boulet ! Désolé de vous avoir dérangé pour pas grand chose. J'aurais tout de même appris quelques trucs au passage.

FileManager::setModule(CloudModule * const) et FileManager::setModule(CloudModule const *) sont différents.
Dans le premier, on a pas le droit de modifier le pointeur, dans le deuxième on a pas le droit de modifier l'objet pointé.
(L'utilisation d'une référence permet de ne pas faire ce genre d'erreur).

Ce serait donc préférable que j'utilise des référence pour mon setModule ?

[Ehonn]

Je n'utilise pas les pointeurs nus en C++ (et encore moins dans l'interface publique d'une classe) car ils sont source d'erreurs et n'ont aucune sémantique (qui alloue ? qui détruit ?).
Donc oui, je te conseille d'utiliser les références : FileManager::setModule(CloudModule const &).

[gilloddon]

Je n'utilise pas les pointeurs nus en C++ (et encore moins dans l'interface publique d'une classe) car ils sont source d'erreurs et n'ont aucune sémantique (qui alloue ? qui détruit ?).
Donc oui, je te conseille d'utiliser les références : FileManager::setModule(CloudModule const &).

Ok, donc si je ne fais pas d'erreur, ça donnerait ça :
- void FileManager::setModule(CloudModule const &); et
- FileManager doit garder un attribut CloudModule const *module; (pointeur quand même, pas référence)
- dans CloudModule : this->fileManager->setModule(*this);

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
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
 
 
class FileManager
{
    public:
        FileManager();
        virtual ~FileManager();
        void setModule(CloudModule const &);
 
    protected:
        CloudModule const *module;
};
 
 
 
FileManager::FileManager()
{
 
}
 
FileManager::~FileManager()
{
 
}
 
void FileManager::setModule(CloudModule const & aModule)
{
    this->module = &aModule;
}
 
 
/* *************** */
 
class CloudModule
{
    public:
        CloudModule();
        virtual ~CloudModule();
 
    private:
        FileManager *fileManager;
};
 
 
 
CloudModule::CloudModule()
{
    this->fileManager = new FileManager();
 
    this->fileManager->setModule(*this);
}
 
CloudModule::~CloudModule()
{
 
}

[jblecanard]

Salut !

Tu sembles progresser dans le bon sens . Si FileManager ne peut pas fonctionner sans le module, alors la référence vers celui-ci devrait être initialisée à la construction du FileManager. Et par extension, sa durée de vie ne peut excéder celle du module qu'il utilise. Par conséquent, tu pourrais utiliser une référence membre (à initialiser dans le constructeur, via la liste d'initialisation) et non un pointeur.

[gilloddon]

J'ai essayé de mettre des références dans FileManager aussi, mais j'ai des erreurs.
J'ai essayé de mettre moins de 'const', mais ça n'arrangeais pas les choses.
Est-ce que tu vois où est ma boulette ?

In constructor 'FileManager::FileManager(const CloudModule&)':
error: uninitialized reference member 'FileManager::module' [-fpermissive]
error: invalid use of incomplete type 'const class CloudModule'
error: forward declaration of 'const class CloudModule'

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
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
 
 
class FileManager
{
    public:
        FileManager(CloudModule const &);
        virtual ~FileManager();
 
    protected:
        CloudModule const & module;
};
 
 
 
FileManager::FileManager(CloudModule const & aModule)
{
    this->module = aModule;
}
 
FileManager::~FileManager()
{
 
}
 
 
/* *************** */
 
class CloudModule
{
    public:
        CloudModule();
        virtual ~CloudModule();
 
    private:
        FileManager *fileManager;
};
 
 
 
CloudModule::CloudModule()
{
    this->fileManager = new FileManager(*this);
 
}
 
CloudModule::~CloudModule()
{
 
}

[jblecanard]

Oui ! Une référence ne peut être initialisée que dans la liste d'initialisation, comme ceci :

Code :

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

FileManager::FileManager(CloudModule const & aModule) : module(aModule) {}

Pour les const, il ne faut pas essayer "à la louche". Si tu connais le sens que tu veux donner à ton code et le rôle de chaque objet, ça se déroulera tout seul. Si FileManager ne doit jamais modifier l'état de CloudModule, alors la référence à CloudModule doit bien être const, et il faut faire fonctionner ton code avec, et pas enlever le const pour voir. Un conseil : prend bien soit de mettre le plus de const possible le plus tôt possible, il est très chiant de devoir les rajouter par la suite si ça n'a pas été bien fait dès le début.

Au passage, je salue ta notation CloudModule const &. Beaucoup (vraiment beaucoup) de code C++ utilise la notation const CloudModule &, qui a le même sens mais qui est abusive car à la base const s'applique sur ce qui se trouve à sa gauche. J'utilise systématiquement la même notation que toi.

[gilloddon]

Merci pour l'explication. Avant aujourd'hui, je pensais que ce type d'initialisation à côté de la signature du constructeur était juste un "raccourci" pour initialiser des attributs ^^

Pour les const, j'essaye effectivement d'en mettre lorsqu'une classe/une méthode ne doit pas modifier les paramètres qu'elles reçoit.


J'y suis donc presque, avec mon bout de code simplifié : dans CloudModule, j'ai un pointeur sur le FileManager et j'essaye de transformer ça en référence. Je coince un peu sur l'initialisation de l'attribut dans le constructeur de CloudModule.

Au début, j'ai fait une boulette en écrivant this->fileManager = FileManager(*this); ^^
Puis je me suis rappelé que la syntaxe était Type maVar; pour instancier statiquement un objet.
J'ai donc essayé FileManager this->fileManager(*this); mais ça bloque.

Est-ce qu'il y a une particularité quand c'est un attribut de classe et non une variable locale ?
Au passage, par rapport à la gestion mémoire, un objet instancié statiquement est normalement détruit lorsqu'il n'est plus visible. Dans ce cas là, vu que c'est un attribut, est-ce qu'il sera bien détruit quand l’instance de la classe sera détruite ou esqt-ce qu'il risque d'y avoir des problèmes ?

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
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
 
class CloudModule;
 
class FileManager
{
    public:
        FileManager(CloudModule const &);
        virtual ~FileManager();
 
    protected:
        CloudModule const & module;
};
 
 
 
FileManager::FileManager(CloudModule const & aModule) : module(aModule)
{
 
}
 
FileManager::~FileManager()
{
 
}
 
 
 
/* *************** */
 
class CloudModule
{
    public:
        CloudModule();
        virtual ~CloudModule();
 
    private:
        FileManager fileManager;
};
 
 
 
CloudModule::CloudModule()
{
    FileManager this->fileManager(*this);
}
 
CloudModule::~CloudModule()
{
 
}

[jblecanard]

Ce problème va se résoudre de la même manière que le précédent : l'usage de la liste d'initialisation. Il faut bien comprendre le rôle de celle-ci, qui porte bien son nom. C'est bien elle qui initialise les membres. Les classes parentes qui n'ont pas de constructeur trivial (constructueur sans argument), les membres qui n'ont pas de constructeur trivial et les références notamment ne peuvent ête initialisés que dans la liste d'initialisation. Une fois que tu te trouves dans le code du constructeur (entre les accolades), l'objet est complètement construit, et tu ne fais que modifier des données membres déjà construites.

Par conséquent, dans l'exemple, tu pourras compiler en écrivant le code ainsi:

Code :

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

CloudModule::CloudModule() : fileManager(*this) {}

Attention cependant ! L'usage de this dans la liste d'initialisation comporte deux pièges qui sont détaillés dans cette entrée de la FAQ. Si l'explication n'est pas claire pour toi, ou si tu fais face à un des problèmes soulevés sans voir comment le résoudre, les solutions existent et nous pourrons te les détailler. Dans ton cas, du moment que le constructeur de FileManager se contente d'initialiser la référence sans l'utiliser dans le code de son constructeur, tout sera OK.

Au passage, par rapport à la gestion mémoire, un objet instancié statiquement est normalement détruit lorsqu'il n'est plus visible. Dans ce cas là, vu que c'est un attribut, est-ce qu'il sera bien détruit quand l’instance de la classe sera détruite ou esqt-ce qu'il risque d'y avoir des problèmes ?

Dans ce cas, l'objet sera correctement détruit. C'est l'un des intérêt du C++ de pouvoir implémenter correctement le RAII.

PS: Note qu'en C++11, les membres peuvent être initialisés dans les headers, bien que cette fonctionalité ne s'applique pas pour le membre fileManager, c'est bon à savoir.

[gilloddon]

Hmmm, du coup avec les const sur la référence, on ne peut pas accéder aux méthodes de l'objet ?
Je pensais que le const, placé tel quel, permettait de s'assurer qu'on ne faisait pas de ré-assignation.

J'ai en effet un problème : dans FileManager, je ne peux pas appeler une méthode du CloudModule via la référence que j'ai en attribut.

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
#ifndef CLOUDMODULE_H
#define CLOUDMODULE_H
 
#include <FileManager.h>
 
class CloudModule
{
    public:
        CloudModule();
        virtual ~CloudModule();
        void moduleOperation();
        int getSomething();
 
    private:
        FileManager fileManager;
};
 
#endif // CLOUDMODULE_H

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
#include "CloudModule.h"
 
 
CloudModule::CloudModule() : fileManager(*this)
{
 
}
 
CloudModule::~CloudModule()
{
 
}
 
void CloudModule::moduleOperation()
{
    this->fileManager.operation();
}
 
int CloudModule::getSomething()
{
    return 777;
}

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
#ifndef FILEMANAGER_H
#define FILEMANAGER_H
 
class CloudModule;
 
class FileManager
{
    public:
        FileManager(CloudModule const &);
        virtual ~FileManager();
        void operation();
 
    protected:
        CloudModule const & module;
};
 
#endif // FILEMANAGER_H

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
#include "FileManager.h"
#include <CloudModule.h>
 
FileManager::FileManager(CloudModule const & aModule) : module(aModule)
{
 
}
 
FileManager::~FileManager()
{
 
}
 
void FileManager::operation()
{
    this->module.getSomething();
}

[jblecanard]

Hello

Si tu peux appeller des fonctions membres de la classe CoudModule... à condition qu'elles soient const elles aussi. Lorsqu'elle sont const, ces fonctions membes s'engagent à ne pas altérer l'état de l'objet. Les getters, notamment, se doivent d'être const.

Code :

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

1
2
3
4
5
6
7
8
9
10
11
class CloudModule
{
    public:
        CloudModule();
        virtual ~CloudModule();
        void moduleOperation();
        int getSomething() const;  // Fonction membre const
 
    private:
        FileManager fileManager;
};

[gilloddon]

Ok, merci pour la précision.


Dernière petite question, je l'espère : pour des références, est-il possible d'empêcher la ré-assignation au sein de la classe seulement ?

>>> Empêcher un this->myRef = anotherRef; en gros, mais sans empêcher l'appel à une méthode sur this->myRef qui pourrait modifier un attribut de myRef.