Re: Problem with static variable definitions

From:

"Alf P. Steinbach" <alfps@start.no>

Newsgroups:

comp.lang.c++

Date:

Fri, 03 Jul 2009 10:13:07 +0200

Message-ID:

<h2ketb$p5g$1@news.eternal-september.org>

* Jaco Naude:

Hi

I'm trying to implement a template based factory, following the
guidelines of the following article:
http://www.codeproject.com/KB/architecture/all_kinds_of_factories.aspx
(I'm trying Factory 6: the template-factory).

I get it to work fine, but as soon as I define more than 1 static
factory, each one in a different class, I get the following error:

Creating library file: bin\libQtilities.a
./tmp\Logger.o: In function
`ZN9Qtilities24AbstractFormattingEngineD1Ev':
D:/ScinericSoftware/Products/Qtilities/trunk/include/Qtilities/
AbstractFormattingEngine.h:(.bss+0x8): multiple definition of
`Qtilities::FileLoggerEngine::factory'

That's probably because you've defined the variable in a header file.

Define it in a separately compiled file.

Or return a reference to it from a function.

./tmp\FileLoggerEngine.o:c:/Tools/Qt/2009.02/qt/include/QtCore/../../
src/corelib/tools/qstring.h:(.bss+0x0): first defined here
collect2: ld returned 1 exit status

Below is some code to show exactly what I'm doing:

    //! Factory interface which is used by factories to create
instances of registered class types.
    /*!
    */
    template <class BaseClass>
    class FactoryInterface
       {
       public:
          FactoryInterface() {}
          virtual ~FactoryInterface() {}
          virtual BaseClass *createInstance() = 0;

This one should probably be 'const'.

virtual QString getTag() = 0;

This one should probably be 'const'.

       };

    //! Factory item class which is used inside classes which can
register themselves as items in factories.
    /*!
    */
    template <class BaseClass,class ActualClass>
    class FactoryItem : public FactoryInterface<BaseClass>
       {
       public:
          FactoryItem() { tag = QString("");}

Just do

FactoryItem(): tag( "" ) {}

or assuming any reasonable default constructor in QString,

FactoryItem() {}

virtual ~FactoryItem() {}
virtual BaseClass *createInstance() {return new
ActualClass;}

With modern compilers you can make that function covariant and thus more useful,

virtual ActualClass* createInstance() const { return new ActualClass; }

Note the 'const'.

QString getTag() { return tag; }

No matter what purpose the "tag" plays, this should probably be 'const'.

          bool setTag(const QString& iface_tag) {
              if (tag == QString("")) {
                  tag = iface_tag;
                  return true;
              } else
                  return false;
          }

This function implies that the "tag" should only be set once.

Why not set it on construction?

And since presumably it would be an error to try to set it more than once, if
you do not set it on construction, throw an exception on attempt to set twice.

       private:
          QString tag;
       };

    //! A factory class which can produce class instances through
registered factory interfaces.
    /*!
    */
    template <class BaseClass>
    class Factory
       {
       public:
          Factory() {}
          ~Factory() {}

Are you sure that this destructor should be empty?

          void registerFactoryInterface(FactoryInterface<BaseClass>*
interface) {
              if (interface->getTag() != QString(""))
                reg_ifaces[interface->getTag()] = interface;
          }

Should failure to register really just be ignored?

void unregisterFactoryInterface(const QString& tag)
{ reg_ifaces.remove(tag); }
QStringList registeredTags() { return reg_ifaces.keys(); }

This one should probably be 'const'.

bool isTagValid(const QString& tag) { return
reg_ifaces.contains(tag); }

This one should probably be 'const'.

          BaseClass* createInstance(const QString& tag) {
              if (isTagValid(tag))
                  return reg_ifaces.value(tag)->createInstance();
              else
                  return 0;
          }

First, this implies that in order to create an instance you need a non-empty tag.

Therefore, that tag *should* be set on construction.

Second, failure to create an instance is serious. It should produce an exception.

       private:
          QMap<QString,FactoryInterface<BaseClass>* > reg_ifaces;
       };

Does the QMap really deallocate the dynamically allocated FactoryInterface
instances?

I then create classes with static FactoryItem factories in them as
shown below:

    class FormattingEngine_Default : virtual public
AbstractFormattingEngine
    {
    public:
        FormattingEngine_Default() : AbstractFormattingEngine() {
            factory_item.setTag("Default");
        }
        ~FormattingEngine_Default() {}

        static FactoryItem<AbstractFormattingEngine,
FormattingEngine_Default> factory_item;

    };

    FactoryItem<AbstractFormattingEngine, FormattingEngine_Default>
FormattingEngine_Default::factory_item;

This one should not be in a header file.

And another class like this:

    class QTILITIES_SHARED_EXPORT FileLoggerEngine : virtual public
AbstractLoggerEngine
    {

    public:
        FileLoggerEngine();
        ~FileLoggerEngine();

        // Make this class a factory item
        static FactoryItem<AbstractLoggerEngine, FileLoggerEngine>
factory;
    };

    FactoryItem<AbstractLoggerEngine, FileLoggerEngine>
FileLoggerEngine::factory;

This one should not be in a header file.

Cheers & hth.,

- Alf