factory, template, et enregistrement des objets

Version imprimable

Voir 40 message(s) de cette discussion en une page

24/08/2012, 13h20
Action Fighter

Salut,

Citation:

Envoyé par 3DArchi

Salut,
Remarques en vrac :
ObjectFactory :
=> la classe FactoryRegister a 2 responsabilités : singleton et fabrique. Il faudrait les dissocier (SRP) en 2 classes différentes.

Je comptai le faire. Je voulais déjà que le principe de la fabrique fonctionne avant de faire ces améliorations.

Citation:

Envoyé par 3DArchi

=> les autres variables membres ne devraient pas être statiques.

=> idem pour les fonctions

Tout à fait, si le singleton et la fabrique sont séparés, il n'y pas d'intérêt à les garder statiques.

Citation:

Envoyé par 3DArchi

=> le double check tel que tu l'implémente ne fonctionne pas en C++03

Là je veux bien plus de précisions. Qu'est-ce qui ne va pas? Ma façon de l'implémenter ou le principe double-check?

Citation:

Envoyé par 3DArchi

=> Plutôt qu'un pointeur de fonction, un std::function offrirait plus de possibilité pour la création des objets

Merci, je ne connaissais pas std::fonction, je vais me pencher dessus.

Citation:

Envoyé par 3DArchi

=> Il n'y a pas besoin de cast lorsqu'on convertit un type depuis une classe dérivée vers une classe de base. C'est le principe même de l'héritage que de pouvoir substituer une instance de la classe dérivée à la classe de base.

C'est noté.

Citation:

Envoyé par 3DArchi

AutoRegister
=> je ne vois pas du tout l'intérêt de l'héritage. Ta macro peut très bien être utilisée pour une déclaration d'une variable privée statique d'auto enregistrement. Cette surcouche d'héritage me semble très fragile

C'est une solution que j'ai déjà essayé mais la macro ne fonctionne pas dans la déclaration de la variable privée statique de classe template.
L'initialisation de la variable statique se faisant après la déclaration de la classe et l'argument passé étant un type template, ce qui est passé la macro n'est pas le type mais juste le nom donné au type dans le template.

Citation:

Envoyé par 3DArchi

Sinon, c'est trop dépendant de Qt à mon goût. La STL contient à peu près tout ce dont tu as besoin si je ne m'abuse.

[/QUOTE]
J'aurai pu faire avec la STL et supprimer la dépendance avec Qt, mais mon but est de faire une librairie ORM utilisant le module QtSql. La librairie sera donc de toute manière dépendante de Qt.
Je pourrais toujours si j'ai un besoin de réutiliser ce code en c++ pur, faire les changements adéquats, ça ne devrai pas être long.
24/08/2012, 15h39
3DArchi

Salut,

Citation:

Envoyé par Action Fighter

Là je veux bien plus de précisions. Qu'est-ce qui ne va pas? Ma façon de l'implémenter ou le principe double-check?

En gros, l'instruction singleton = new Type; n'est pas atomique. Le pointeur peut alors prendre une valeur alors que la construction de l'objet n'est pas terminée.
Un peu de lecture :
C++ and The Perils of Double-Checked Locking: Part I By Scott Meyers and Andrei Alexandrescu
C++ and the Perils of Double-Checked Locking: Part II

En C++11, cf Multithreading in C++0x part 6: Lazy initialization and double-checked locking with atomics d'Anthony Williams

Citation:

Envoyé par 3DArchi

=> Il n'y a pas besoin de cast lorsqu'on convertit un type depuis une classe dérivée vers une classe de base. C'est le principe même de l'héritage que de pouvoir substituer une instance de la classe dérivée à la classe de base.

En lisant son code je pensais qu'il voulait s'assurer que le type instancié héritait bien du type de base associé au template :
Code:

1 2 3 4 5 6 7 8 template<typename T_OBJECT> static T_BASE* derivedClassCreationFunc() { T_OBJECT* t = new T_OBJECT; T_BASE* c = dynamic_cast<T_BASE*>(t); if (!c) delete t; return c; }
quelle était ton but avec ce cast Action Fighter ?

edit: en même temps j'avais zappé le type de retour

24/08/2012, 16h22
Flob90
En C++11 :
Code:

1 2 3 4 5 6 7 template<class T> T& Singleton<T>::get_instance() { static T _instance; return _instance; }
Convient pour un singleton thread-safe, inutile d'aller chercher du double-check, CAS, atomic, ...
24/08/2012, 16h29
3DArchi
Citation:
Envoyé par Flob90

En C++11 :

Code:

1 2 3 4 5 6 7 template<class T> T& Singleton<T>::get_instance() { static T _instance; return _instance; }

Convient pour un singleton thread-safe, inutile d'aller chercher du double-check, CAS, atomic, ...
A ma connaissance, c'est safe aussi en C++03.
La problématique du double-check n'intervient que lorsqu'on veut allouer dynamiquement l'instance.
24/08/2012, 16h45
Flob90
Non, c'est nouveau (changement au 4° paragraphe du 6.7 de la norme, entre autre).

Pour une allocation dynamique :
Code:

1 2 3 4 5 6 7 template<class T> T& Singleton<T>::get_instance() { static T* _instance { new T{} }; return *_instance; }
Ce n'est qu'un morceau de code isolé, une version plus complète pourra faire intervenir d'autre chose. Typiquement pour faire une gestion plus fine : phoenix singleton par exemple.

Le fait que ce "type" de code en C++11 soit thread-safe rend totalement trivial la résolution de la problématique de la création d'un objet dans un contexte d'appel concurrent. Ce qui est bien ce a quoi devait répondre les anciennes solutions : double-check, CAS, atomic, ...
24/08/2012, 16h53
3DArchi

Ok. Tu fais référence à cette phrase

Citation:

If control enters
the declaration concurrently while the variable is being initialized, the concurrent execution shall wait for completion of the initialization.

je suppose.

Effectivement, après il n'y a plus trop de pb.

Merci pour ces réponses.

Après quelques essais, j'ai pris les solutions suivantes :

- Pour ObjectFactory, plutôt qu'un singleton, j'en ai fait une classe monostate. Donc plus de problème de thread-safety, les méthodes de QHash étant déjà thread-safe.

- Pour std::function, pas dispo avec le sdk de Qt. Je vais donc garder les pointeurs de fonctions.

Dans derivedClassCreationFunc() d'ObjectFactory, je voulais bien m'assurer que le type dérivé soit bien un dérivé du type de base.

Voici mon code :

ObjectFactory.h

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
 
#ifndef OBJECTFACTORY_H
#define OBJECTFACTORY_H
 
#include <QHash>
 
template<typename T_ID, typename T_BASE>
class ObjectFactory
{
    typedef T_BASE* (ClassCreationFunc)();
    typedef ObjectFactory<T_ID,T_BASE> factory;
    typedef QHash<T_ID, ClassCreationFunc*> factoryQHash;
 
    static factoryQHash s_registry;
 
public :
    ObjectFactory() {}
 
    template<typename T_OBJECT>
    static T_BASE* derivedClassCreationFunc()
    {
        T_OBJECT* t = new T_OBJECT;
        T_BASE* c = static_cast<T_BASE*>(t);
        if (!c) delete t;
        return c;
    }
 
    inline void registerObject(const T_ID& id, ClassCreationFunc* func)
    {
        s_registry.insert(id, func);
    }
 
    inline T_BASE* create(const T_ID& id)
    {
        T_BASE* obj = 0;
        typename factoryQHash::iterator it = s_registry.find(id);
        if (it != s_registry.end()) {
          ClassCreationFunc* pFunc = it.value();
          obj = pFunc();
        }
        return obj;
    }
 
    template<typename T_OBJECT>
    inline T_OBJECT* create(const T_ID& id)
    {
        T_BASE* obj = 0;
        typename factoryQHash::iterator it = s_registry.find(id);
        if (it != s_registry.end()) {
          ClassCreationFunc* pFunc = it.value();
          obj = pFunc();
        }
        return static_cast<T_OBJECT*> (obj);
    }
};
template<typename T_ID, typename T_BASE> typename ObjectFactory<T_ID, T_BASE>::factoryQHash ObjectFactory<T_ID, T_BASE>::s_registry;
 
#endif // OBJECTFACTORY_H

FactoryRegister.h

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
 
#ifndef FACTORYREGISTER_H
#define FACTORYREGISTER_H
 
#include "objectfactory.h"
 
#define TYPETOSTRING(Classname) #Classname
 
template<typename typeBase, typename ClassName>
struct FactoryRegister
{
    typedef ObjectFactory<QString,typeBase> Factory;
    typedef FactoryRegister<typeBase,ClassName> factoryRegister;
 
    FactoryRegister() {
        Factory fac;
        fac.registerObject(TYPETOSTRING(Classname), Factory::template derivedClassCreationFunc<ClassName>);
    }
};
 
template<typename typeBase, typename ClassName>
struct AutoRegister : public typeBase
{
    typedef FactoryRegister<typeBase,ClassName> factoryRegister;
 
    AutoRegister() {}
    static factoryRegister s_factoryregister;
 
    static ClassName* create() {
        ObjectFactory<QString,ClassName> fac;
        return static_cast<ClassName*> (fac.create(TYPETOSTRING(ClassName)));
    }
};
template<typename typeBase, typename ClassName>
FactoryRegister<typeBase,ClassName> AutoRegister<typeBase,ClassName>::s_factoryregister;
 
#endif // FACTORYREGISTER_H

AbstractObject.h

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
 
#ifndef ABSTRACTOBJECT_H
#define ABSTRACTOBJECT_H
 
#include "factoryregister.h"
 
class AbstractObject
{
public:
    typedef ObjectFactory<QString,AbstractObject> Factory;
 
    AbstractObject() {}
    virtual ~AbstractObject() {}
 
#define REGISTER_OBJECT(ClassName) public AutoRegister<AbstractObject,ClassName>
};
 
#endif // ABSTRACTOBJECT_H

TestObject.h

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
#ifndef TESTOBJECT_H
#define TESTOBJECT_H
 
#include "abstractobject.h"
 
class TestObject : REGISTER_OBJECT(TestObject)
{
public:
    TestObject();
    ~TestObject() {}
 
    void affiche(QString testString) {qDebug(testString.toAscii());}
};
 
#endif // TESTOBJECT_H

main.cpp

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
#include <QtCore/QCoreApplication>
#include "testobject.h"
 
int main(int argc, char *argv[])
{
    QCoreApplication a(argc, argv);
 
    TestObject* testObject = TestObject::create();
 
    testObject->affiche("Hello");
 
    return a.exec();
}

Maintenant, je pense améliorer le tout en permettant d'enregistrer une autre fonction de création.

Salut,

Citation:

Envoyé par Action Fighter

- Pour ObjectFactory, plutôt qu'un singleton, j'en ai fait une classe monostate. Donc plus de problème de thread-safety, les méthodes de QHash étant déjà thread-safe.

Singleton et monostate, c'est bonnet blanc ou blanc bonnet. Les problèmes sont éventuellement déplacés mais ne disparaissent certainement pas.

Citation:

Envoyé par Action Fighter

- Pour std::function, pas dispo avec le sdk de Qt. Je vais donc garder les pointeurs de fonctions.

1/ Boost.Function

2/ Au pire, un foncteur on ne peut plus classique :
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 template<class T> struct creator{ T*operator()() const { return do_create(); } virtual ~creator(){} private: virtual T* do_create() const=0; }; template<class T, class Base> struct creator_derived : public creator<Base>{ private: virtual T* do_create() const {return new T;} }; struct AbstractObject { virtual ~AbstractObject()=0; }; AbstractObject::~AbstractObject(){} struct Derived_1 : public AbstractObject {}; struct Derived_2 : public AbstractObject {}; #include <map> #include <string> int main() { std::map<std::string,creator<AbstractObject>* > fac; fac["Derived_1"] = new creator_derived<Derived_1,AbstractObject>; fac["Derived_2"] = new creator_derived<Derived_2,AbstractObject>; AbstractObject *p1 = (*fac["Derived_1"])(); AbstractObject *p2 = (*fac["Derived_2"])(); return 0; }
L'exemple est avec des pointeurs nus mais, bien sur, tu utilises les pointeurs intelligents adéquats de Qt (l'équivalent d'un std::unique_ptr avec une sémantique de mouvement serait plus adéquat qu'un shared ptr. Ceci dit, faute de grives, on mange des merles => shared_ptr).

3/ J'ai pas mené l'analyse complète mais je pense que ton code ne fonctionne pas pour au moins une raison : l'ordre d'initialisation des variables statiques entre différentes unités de compilation est indéterminé. Ce qui veut dire que tu peux potentiellement avoir un s_factoryregister construit avant que le constructeur de s_registry ne soit appelé.

Salut,

Citation:

Envoyé par 3DArchi

Salut,

Singleton et monostate, c'est bonnet blanc ou blanc bonnet. Les problèmes sont éventuellement déplacés mais ne disparaissent certainement pas.

Je ne comprend pas ta remarque. Il n'y a plus de problème de création d'instance unique, puisque plusieurs ObjectFactory peuvent être instanciées. Ensuite, les accès au registre qui est un type interne à Qt sont déjà thread-safe.

Citation:

Envoyé par 3DArchi

1/ Boost.Function

2/ Au pire, un foncteur on ne peut plus classique :

L'exemple est avec des pointeurs nus mais, bien sur, tu utilises les pointeurs intelligents adéquats de Qt (l'équivalent d'un std::unique_ptr avec une sémantique de mouvement serait plus adéquat qu'un shared ptr. Ceci dit, faute de grives, on mange des merles => shared_ptr).

Je n'ai pas envie d'ajouter une dépendance à Boost, mais je testerai avec des foncteurs.

Citation:

Envoyé par 3DArchi

3/ J'ai pas mené l'analyse complète mais je pense que ton code ne fonctionne pas pour au moins une raison : l'ordre d'initialisation des variables statiques entre différentes unités de compilation est indéterminé. Ce qui veut dire que tu peux potentiellement avoir un s_factoryregister construit avant que le constructeur de s_registry ne soit appelé.

Ce code fonctionne sous GCC, je n'ai pas testé avec un autre compilateur, mais cela devrait fonctionner.
Quand l'objet s_factoryregister est construit, son constructeur instancie une factory pour faire l'enregistrement, le constructeur de s_registry est donc appelé pendant la construction de s_factoryregister, ce qui permet de faire l'enregistrement de l'objet dérivé.

Par contre, une chose que je ne comprend pas, je m'aperçois que le constructeur de TestObject n'est jamais appelé.
Code:

1 2 3 4 5 6 7 TestObject::TestObject() { // Ce code n'est jamais appelé QString test = "L'objet TestObect a ete cree"; qDebug(test.toStdString().c_str()); }
Ce code n'est jamais appelé, alors que je peux accéder aux méthodes de TestObject une fois créé par la factory
Code:

1 2 3 4 5 6 7 8 9 10 11 int main(int argc, char *argv[]) { QCoreApplication a(argc, argv); TestObject* testObject = TestObject::create(); testObject->affiche("Hello"); // Ce code appelle bien la méthode et affiche bien "Hello" return a.exec(); }

Citation:

Envoyé par Action Fighter

l n'y a plus de problème de création d'instance unique, puisque plusieurs ObjectFactory peuvent être instanciées. Ensuite, les accès au registre qui est un type interne à Qt sont déjà thread-safe.

Singleton et Monostate s'appuient tous les deux sur des variables globales. Et c'est le fait d'avoir des variables globales qui pose les problèmes de thread-safety (entre autres).

Citation:

Envoyé par Action Fighter

Je n'ai pas envie d'ajouter une dépendance à Boost

C'est ce que j'avais cru comprendre, c'est pourquoi j'ai proposé les foncteurs à la main. Néanmoins, je pense que puisque tu n'es pas sous C++11, ce choix te prive de bibliothèques très utiles. Tu sais, ce n'est pas un hasard si celles dont tu as besoin (boost::function, boost::bind principalement) font parties de celles intégrées au langage via la STL. Cela montre que ce besoin est plus que crucial. Enfin, ces deux bibliothèques n'utilisent que des fichiers d'en-têtes (pas de .lib/.dll/.so) et sont donc faciles à intégrées.

Citation:

Envoyé par Action Fighter

Ce code fonctionne sous GCC, je n'ai pas testé avec un autre compilateur, mais cela devrait fonctionner.

Le propre d'un comportement indéterminé est qu'il peut 'tomber en marche' sur un exemple mais, plus tard, donner quelque chose de complètement différent. Que va-t-il se passer quand tu auras plusieurs objet dérivants de AbstractObject dans des .cpp différents ? Cela fonctionne-t-il encore en mode release avec des optimisations agressives ? As-tu testé avec d'autres compilateurs ? Que se passe-t-il si tu as plusieurs factory sur des hierarchies différentes et chacune avec plusieurs classes dérivées le tout mixé dans des unités de compilation différentes ? Que se passe-t-il si les fichiers sont compilés dans des ordres différents ?

Citation:

Envoyé par Action Fighter

Quand l'objet s_factoryregister est construit, son constructeur instancie une factory pour faire l'enregistrement, le constructeur de s_registry est donc appelé pendant la construction de s_factoryregister, ce qui permet de faire l'enregistrement de l'objet dérivé.

template<typename T_ID, typename T_BASE> typename ObjectFactory<T_ID, T_BASE>::factoryQHash ObjectFactory<T_ID, T_BASE>::s_registry est une variable globale.
template<typename typeBase, typename ClassName> FactoryRegister<typeBase,ClassName> AutoRegister<typeBase,ClassName>::s_factoryregister; aussi.
Or la norme dit que l'ordre d'appel des constructeurs de 2 variables globales dans des unités de compilation (~fichiers .cpp) différentes est indéterminé. Donc lorsque s_factoryregister instancie un Factory et utilise la fonction d'enregistrement s_registry peut ne pas avoir été construit et aboutir à un plantage.
Citation:
Envoyé par Action Fighter
Par contre, une chose que je ne comprend pas, je m'aperçois que le constructeur de TestObject n'est jamais appelé.
Code:

1 2 3 4 5 6 7 TestObject::TestObject() { // Ce code n'est jamais appelé QString test = "L'objet TestObect a ete cree"; qDebug(test.toStdString().c_str()); }
Ce code n'est jamais appelé, alors que je peux accéder aux méthodes de TestObject une fois créé par la factory
Code:

1 2 3 4 5 6 7 8 9 10 11 int main(int argc, char *argv[]) { QCoreApplication a(argc, argv); TestObject* testObject = TestObject::create(); testObject->affiche("Hello"); // Ce code appelle bien la méthode et affiche bien "Hello" return a.exec(); }
Mets un point d'arrêt et regarde ce qu'il se passe.

31/08/2012, 08h31
Joel F

Y a aussi la solution de pas faire de singleton pour etre propre et testable :o

Ensuite pas vouloir une dependance à Bosot alors qu'ont riane le boulet astronomique de Qt me fera toujours rire :lol:

Voir 40 message(s) de cette discussion en une page