Discussion :

[Action Fighter]

Salut,

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.

=> 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.

=> 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?

=> 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.

=> 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é.

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.

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.

[3DArchi]

Salut,

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

[Aurelien.Regat-Barrel]

=> 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 :

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

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

[Flob90]

En C++11 :

Code :

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

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, ...

[3DArchi]

En C++11 :

Code :

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

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.

[Flob90]

Non, c'est nouveau (changement au 4° paragraphe du 6.7 de la norme, entre autre).

Pour une allocation dynamique :

Code :

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

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, ...

[3DArchi]

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

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.

[Action Fighter]

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 :

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
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 :

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
 
#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 :

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 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 :

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

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 :

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

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.

[3DArchi]

Salut,

- 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.

- 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 :

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
 
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é.

[Action Fighter]

Salut,

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.

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.

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 :

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

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 :

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

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();
}

[3DArchi]

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).

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.

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 ?

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.

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

Code :

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

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 :

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

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.

[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