8

These functions are Utility type things that most of my programs objects will use. I want to have them in a namespace and have them global. This namespace is defined in a header and then added to my precompiled header. However so far I have used the function from this namespace in 2 different objects and the compiler is throwing the multiply defined symbols error on these 2 objects.

namespace file

#ifndef UTILS_H
#define UTILS_H

#include <random>
#include <cmath>


namespace Utils
{
    extern int GetRandomBetween(int low, int high)
    {
        if (low < 0 || low >= high)
            return 0;
        int seed = high - low;

        return (rand() % seed) + low;
    }
};

#endif

and my precomp header

// stdafx.h : include file for standard system include files,
// or project specific include files that are used frequently, but
// are changed infrequently
//

#pragma once

#include "targetver.h"

//#define WIN32_LEAN_AND_MEAN             // Exclude rarely-used stuff from Windows headers
// Windows Header Files:
#include <windows.h>

// C RunTime Header Files
#include <stdlib.h>
#include <malloc.h>
#include <memory.h>
#include <tchar.h>
#include <random>


#define SAFE_DELETE( p )       { if( p ) { delete ( p );     ( p ) = NULL; } }
#define SAFE_DELETE_ARRAY( p ) { if( p ) { delete[] ( p );   ( p ) = NULL; } }
#define SAFE_RELEASE( p )      { if( p ) { ( p )->Release(); ( p ) = NULL; } }
// TODO: reference additional headers your program requires here

#include "Utils.h"
#include "Manager.h" // this object uses utils
#include "Bot.h"    // this object uses utils
#include "LinkedList.h"
#include "Village.h"  // this object will use utils in the future

The linker error message:

Manager.obj : error LNK2005: "int __cdecl Utils::GetRandomBetween(int,int)" (?GetRandomBetween@Utils@@YAHHH@Z) already defined in Bot.obj stdafx.obj : error LNK2005: "int __cdecl Utils::GetRandomBetween(int,int)" (?GetRandomBetween@Utils@@YAHHH@Z) already defined in Bot.obj c:\users\lee\documents\visual studio 2010\Projects\AI\Debug\AI.exe : fatal error LNK1169: one or more multiply defined symbols found

it also maybe worth noting that in my Manager class header I forward declared Bot. Same with Village class header.

J. Chomel
  • 8,193
  • 15
  • 41
  • 69
EddieV223
  • 5,085
  • 11
  • 36
  • 38

2 Answers2

11

Your function definition (ie: the source code) should not be in a header. The reason you are getting multiple definitions is that extern cannot convert a function definition (the source code) into a function declaration (ie: just the prototype). So you need to do this:

Util.h:

namespace Utils
{
    int GetRandomBetween(int low, int high);
};

SomeSourceFile.cpp (probably Util.cpp):

namespace Utils
{
    int GetRandomBetween(int low, int high);
    {
        if (low < 0 || low >= high)
            return 0;
        int seed = high - low;

        return (rand() % seed) + low;
    }
};

Alternatively, you can declare the function inline in the header:

namespace Utils
{
    inline int GetRandomBetween(int low, int high)
    {
        if (low < 0 || low >= high)
            return 0;
        int seed = high - low;

        return (rand() % seed) + low;
    }
};

Though you should only use this for small functions.

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
2

Manager.cpp and Bot.cpp both include Util.h

Because of this, when they are compiled, both object files export the symbol "GetRandomBetween". When the linker goes to combine these object files into an executable, it finds 2 instances of the function. The linker can't determine which one to use (and it doesn't understand that they are identical).

If you want to make the object files NOT export the symbol (so that you dont have a linker conflict), remove the extern keyword.

J T
  • 4,946
  • 5
  • 28
  • 38
  • It doesn't work without it. I had written it without the extern keyword first and thats when I got the error and added the exern. But I went ahead and tried it again without extern and still doesn't work same error. – EddieV223 Aug 05 '11 at 22:15
  • 3
    @J's analysis is correct, but his advice is wrong. You have two choices: either merely declare the function in the header, and move the definition to a CPP file, or replace `extern` with `inline`. Okay, three choices -- you could replace `extern` with `static`, but don't. `inline` will be less wasteful than `static`. – Robᵩ Aug 05 '11 at 22:21
  • Perfect! Thank you, rep for u both. – EddieV223 Aug 05 '11 at 22:24
  • Ah, you guys saved my day! `inline` works like a charm! But @Rob, while we are at it, why will `inline` be less wasteful than `static`? Why would any of these options will be wasteful at all? – SexyBeast Sep 23 '16 at 21:42
  • @SexyBeast Both static and inline will create 2 copies of the fn, but they'll will be accessible only to the translation units that they are defined within (a.obj, b.obj) - that's why this solution solves the error. Difference between the two is inline will embed the code where its called, but static will add a jump assembly instruction and make a stack frame. The frame and jump is runtime expensive, but creates a more compressed binary. The inline method will bloat your binary but optimized runtime cost. A modern compiler will ignore both and optimize your code with what it thinks is best. – J T Oct 19 '16 at 15:19