0

i have this problem where i cannot compile if i include a certain header.

The code for the header:

//#include "tinyxml2.h"

#pragma once

#include "Camera.h"
#include "Controller.h"
#include "Lights.h"
#include "Mesh.h"

namespace ActorFactory {

    //using namespace tinyxml2;

    template<class T>  
    Actor* createInstance() { 
        return new T; 
    }

    typedef std::map<std::string, Actor*(*)()> class_map;
    class_map test = {
        { "Camera", &createInstance<Camera> }
        //{ "Mesh", &createInstance<Mesh> }
    };
}

The full error message from visual studio:

1>------ Build started: Project: ogl_inf251_ca2, Configuration: Debug Win32     ------
1>  Main.cpp
1>d:\development\inf251\ogl_inf251_ca2\model_obj.h(26): warning C4005:     '_CRT_SECURE_NO_WARNINGS' : macro redefinition
1>          command-line arguments :  see previous definition of     '_CRT_SECURE_NO_WARNINGS'
1>  Generating Code...
1>  Compiling...
1>  Scene.cpp
1>d:\development\inf251\ogl_inf251_ca2\model_obj.h(26): warning C4005:     '_CRT_SECURE_NO_WARNINGS' : macro redefinition
1>          command-line arguments :  see previous definition of     '_CRT_SECURE_NO_WARNINGS'
1>  Generating Code...
1>Scene.obj : error LNK2005: "class std::map<class     std::basic_string<char,struct std::char_traits<char>,class std::allocator<char>     >,class Actor * (__cdecl*)(void),struct std::less<class     std::basic_string<char,struct std::char_traits<char>,class std::allocator<char>     > >,class std::allocator<struct std::pair<class std::basic_string<char,struct     std::char_traits<char>,class std::allocator<char> > const ,class Actor *     (__cdecl*)(void)> > > ActorFactory::test" (?test@ActorFactory@@3V?$map@V?    $basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@P6APAVActor@@XZU?    $less@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@2@V?    $allocator@U?$pair@$$CBV?$basic_string@DU?$char_traits@D@std@@V?    $allocator@D@2@@std@@P6APAVActor@@XZ@std@@@2@@std@@A) already defined in     Main.obj
1>D:\development\inf251\Debug\ogl_inf251_ca2.exe : fatal error LNK1169: one     or more multiply defined symbols found
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

The error message is really long unfortunately. I have no idea how to fix this or even whats wrong. Also is there a better way to instantiate classes by name?

Thanks for any help in advance :)

Jens
  • 113
  • 1
  • 10
  • This defines a variable `ActorFactory::test` in a header. Include that header in more than one `.cpp` file (directly or otherwise) and you're going to get exactly what this error message is telling you: a multiple definitions encounter. So... don't do that. – WhozCraig Mar 22 '15 at 21:03
  • This header is only included in one other header. But the Camera, Controller and the others are included in multiple. Is that a problem? – Jens Mar 22 '15 at 21:37

4 Answers4

5

Your test object violates the one-definition rule.

The #pragma once keeps the definition from appearing multiple times in the same compilation unit (read: the same *.cpp file), but not from appearing within different compilation units (read: different *.cpp files).

One way to fix this problem is to convert the object into a static local object within a function and make it accessible via a reference:

header.h:

// ...

namespace ActorFactory {

    // ...

    typedef std::map<std::string, Actor*(*)()> class_map;
    class_map& test();
}

test.cpp:

#include "header.h"

namespace ActorFactory {

    // ...

    class_map& test() {
        static class_map test = {
            { "Camera", &createInstance<Camera> }
            { "Mesh", &createInstance<Mesh> };
        return test;
    }
}

This is also a fail-safe guarantee against the so-called static initialization order fiasco, and gives you lazy initialisation (not that it matters much with this tiny object).

You can make the whole thing even safer (in the face of order-of-destruction issues) by allocating the object with new and never deleting it, but the function should still return a reference:

namespace ActorFactory {

    // ...

    class_map& test() {
        static class_map* test = new class_map {
            { "Camera", &createInstance<Camera> }
            { "Mesh", &createInstance<Mesh> };
        return *test;
    }
}

Read the FAQs I linked to above to get all the details.

In any case, client code then simply accesses the object via test() instead of test.

Christian Hackl
  • 27,051
  • 3
  • 32
  • 62
  • Hey, thanks for the answer! Your solution works :) But I'm not entirely sure i understand why. Is it because Camera.cpp, Controller.cpp, Lights.ccp, Mesh.cpp and any other indirectly included class could be compiled after Scene.cpp (Which includes ActorFactory.h)? – Jens Mar 22 '15 at 21:48
  • @user3591315: Actually, I just noticed a mistake in my answer. Let me fix it first... – Christian Hackl Mar 22 '15 at 21:51
  • @user3591315: The solution works because the object is no longer at namespace scope. It is now created within a function, and of the function, only the *declaration* appears in the header, not the *definition* (this keeps the *function* from violating ODR). The order in which *.cpp files are compiled is not defined, so "compiled after Scene.cpp" is a meaningless or dangerous phrase in C++. As I said elsewhere, you probably included this `ActorFactory.h` (which I apparently incorrectly called `header.h` in my answer) indirectly in some other *.cpp file... – Christian Hackl Mar 22 '15 at 21:58
3

The trouble is the way you have declared test.

You can either declare it static or const (which implies static in this case). However, you will have a separate copy of test for every source file which includes this header, wasting memory and possibly causing other issues.

Therefore you should declare your test in the header extern class_map test; and in only one source file, put ActorFactory::class_map ActorFactory::test = {...}; (If you make it const, put extern in the source file bit too).

Neil Kirk
  • 21,327
  • 9
  • 53
  • 91
  • This also works. But I'm still not sure why. ActorFactory.h is only included by Scene.h, which inturn is included by Main.cpp. Camera.h is included in both Scene.h and ActorFactory.h. Could this be a problem? – Jens Mar 22 '15 at 21:55
  • @user3591315 There's no point trying to understand why some random header include combinations work or not if the variable wasn't declared the right way. – Neil Kirk Mar 22 '15 at 21:58
  • For me, this boils down to: Rule 1: don't ever initialise things in a header file: there are too many problems resulting. Only ever put declarations in headers. Rule 2: try very hard not to use static data. Ever. It just causes pain. – rivimey Mar 22 '15 at 22:18
  • @rivimey I am ok with basic type consts in header files but as you say, other things cause too much trouble. – Neil Kirk Mar 22 '15 at 22:19
  • Neil - I agree numbers are ok; even strings have their problems (where are they stored?) – rivimey Mar 22 '15 at 23:30
  • @rivimey What type of string? – Neil Kirk Mar 23 '15 at 00:06
  • Neil, if you do char *s="hello" as a static alloc, then "hello" gets compiled into the object. With multiple inclusions of the file you get multiple strings stored. Some linkers will coalesce such strings, but not all. – rivimey Mar 23 '15 at 01:31
  • I don't know what you mean by "static alloc", it's true however the string literal appears. – Neil Kirk Mar 23 '15 at 01:43
0

It seams to me that you instanciate the 'test' variable inside a header, which is include from Main.Cpp and Scene.Cpp. No ?

If your header is included from two .cpp files, it will results in duplicated symbols. #pragma once only prevent a header from being included more than once when compiling the same source file, bu it still can be included a second time when compiling another source code.

etham
  • 3,158
  • 2
  • 16
  • 13
  • Its only included in Scene.h, but Controller, Camera and all that is included in both Scene.h and ActorFactory.h. Is that a problem? – Jens Mar 22 '15 at 21:33
  • @user3591315: Not sure I understand your comment, but it does not matter whether a header file is included directly or indirectly (through other headers). – Christian Hackl Mar 22 '15 at 21:34
  • Okei, because Scene is included in main, and Scene includes ActorFactory. But thats it though. :o – Jens Mar 22 '15 at 21:39
  • @user3591315: Well, I guess you also have something like `Scene.cpp` which includes `Scene.h`. You *must* have some other *.cpp file in your project which somehow includes `ActorFactory.h` (directly or indirectly). Otherwise, the linker error would not make sense. – Christian Hackl Mar 22 '15 at 21:46
  • @ChristianHackl Yes i have a Scene.cpp which includes Scene.h – Jens Mar 22 '15 at 21:55
  • @user3591315: Well, then the `test` object from your original code *was* defined multiple times, in different compilation units: one time in `Scene.cpp`, another time in `main.cpp`. Which is exactly what both mine and Neil Kirk's answer were trying to explain. The scope of `#pragma once` (and the preprocessor in general) is a single compilation unit, not the whole program. – Christian Hackl Mar 22 '15 at 22:07
-1

It looks like its complaining about a redefinition of test. Try renaming it?

Ron Thompson
  • 1,086
  • 6
  • 12