How can I be getting "already defined" linker errors here?

Question

I'm making my first attempt at unit testing in C++, and I haven't used C++ in a number of years (I'm mainly a C# coder at the moment). It seems like I'm making a right pig's ear of it - I hope someone can steer me back onto the righteous path. I'm just getting started here and would really like to be implementing these tests using the best practice possible, so any and all comments are welcome, even though I'm most concerned with my linker error at present.

So, I have an overall solution "Technorabble", with sub-projects "CalibrationTool" and "CalibrationToolUnitTests".

CalibrationTool has a MathUtils.h file:

#ifndef __math_utils__
#define __math_utils__

#include "stdafx.h"
#include <vector>

namespace Technorabble
{
    namespace CalibrationTool
    {
        double GetDoubleVectorAverage(std::vector<double> v)
        {
            double cumulativeValue = 0;
            for(std::vector<double>::iterator iter = v.begin(); iter != v.end(); ++iter) 
            {
                cumulativeValue += *iter;
            }
            return cumulativeValue / v.size();
        }
    }; // end namespace CalibrationTool
};  // end namespace Technorabble

#endif // !__math_utils__

(But no .cpp file as I was having all kinds of (somewhat similar) issues getting my template function working - so I ended up defining that inline).

Moving on to the Unit Tests project, I have a main.cpp:

#include "MathUtilsTest.h"

void RunMathUtilsTests();

int main()
{
    RunMathUtilsTests();

    // Other class tests will go here when I have things to test
}

void RunMathUtilsTests()
{
    MathUtilsTest* mathUtilsTest = new MathUtilsTest();
    mathUtilsTest->RunTests();
    delete mathUtilsTest;
}

Finally, the header and cpp for the MathUtilsTest class, again, fairly simple:

.h:

#ifndef __MATH_UTILS_TEST__
#define __MATH_UTILS_TEST__

#include "CalibrationToolUnitTestsLogging.h"
#include "..\CalibrationTool\MathUtils.h"

class MathUtilsTest
{
public:
    MathUtilsTest();
    ~MathUtilsTest();

     bool RunTests();

private:    
    bool GetDoubleVectorAverageTest();

}; // end class MathUtilsTest

#endif

.cpp:

#include "MathUtilsTest.h"
#include <sstream>

bool MathUtilsTest::RunTests()
{
    return GetDoubleVectorAverageTest();

}

MathUtilsTest::~MathUtilsTest()
{

}

MathUtilsTest::MathUtilsTest()
{

}

bool MathUtilsTest::GetDoubleVectorAverageTest()
{
    bool passed = true;
    std::vector<double> values;
    for (int i = 1; i < 23; i++)
    {
        values.push_back(i);
    }
    // vector becomes: 1, 2, 3, 4, .....20, 21, 22.  Average is 11.5
    double expectedAverage = 11.5;
    double calculatedAverage = Technorabble::CalibrationTool::GetDoubleVectorAverage(values);
    if (calculatedAverage != expectedAverage)
    {
        std::ostringstream s;
        s << calculatedAverage;
        std::string avgString = s.str();
        CalibrationToolUnitTestsLogging::Write("Failed MathUtilsTest.GetDoubleVectorAverageTest:  " + avgString);
        passed = false;
    }
    else
    {
        CalibrationToolUnitTestsLogging::Write("Passed MathUtilsTest.GetDoubleVectorAverageTest");
    }

    return passed;
}

This all seemed fine to me, I'm protecting my header with #ifndef, etc. But I'm still getting the following errors:

1) error LNK1169: one or more multiply defined symbols found

2) error LNK2005: "double __cdecl Technorabble::CalibrationTool::GetDoubleVectorAverage(class std::vector >)" (?GetDoubleVectorAverage@CalibrationTool@Technorabble@@YANV?$vector@NV?$allocator@N@std@@@std@@@Z) already defined in main.obj C:_SVN\Technorabble\Windows Software\CalibrationToolUnitTests\MathUtilsTest.obj

How can this be? Can anyone spot where it's going wrong?

Answer 1

You are defining a function GetDoubleVectorAverage in a header. This means that it will be defined in every translation unit (i.e. every source file) that includes that header. If your program contains more than one such translation unit, then you'll have more than one definition - which isn't allowed.

Solutions are:

• Add inline to the function definition, to relax this rule and allow multiple identical definitions; or
• Move the function definition into a source file, and only declare it in the header.

I'm protecting my header with #ifndef

That only prevents the header from being included more than once within the same translation unit. It doesn't prevent inclusion from more than one unit.

Also, you shouldn't use a reserved name like __math_utils__ as a header guard, even if the internet is littered with examples of dodgy code doing that.

I was having all kinds of (somewhat similar) issues getting my template function working

Templates usually need to be defined in header files, to make the definition available at the point of use. Function templates are implicitly inline, but normal functions (like this one) aren't.

Answer 2

Functions defined in headers should be marked as inline:

inline double GetDoubleVectorAverage(std::vector<double> v)
{
}

If it's longer than a couple of lines, consider moving it to an implementation file.

pragmas or include guards don't protect against multiple definitions.

Note that you should pass v by const reference rather than by-value.