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?

Not related to your linker problem, but have you considered using an actual unit testing framework? That'll usually be a much better solution than rolling your own. — Crowman, Sep 17 '13 at 16:34

score 5 · Accepted Answer · answered Sep 17 '13 at 14:28

5

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.

answered Sep 17 '13 at 14:28

Luchian Grigore

253,575
64
457
625

Thanks, inline seems to work (although moving the code the the .cpp does not). I have introduced a new issue with const though. My iterator has become invalid. "Cannot convert from std::vector_const_iterator to std::vector_iterator. I'll google it. – technorabble Sep 17 '13 at 14:38
@technorabble: If the vector is const, then you'll have to change the iterator type to `std::vector::const_iterator`. In C++11, you can use `auto` to save typing. – Mike Seymour Sep 17 '13 at 14:43
Thanks, fixed. Moving the implementation to MathUtils.cpp seems like it will be needed if not here, then certainly for larger functions down the line, but when I do this I get an unresolved external symbol error. Is there any obvious reason this would happen? – technorabble Sep 17 '13 at 15:13
@mike-seymour: Here's the error,not sure if that sheds additional light? error LNK2019: unresolved external symbol "double __cdecl Technorabble::CalibrationTool::GetDoubleVectorAverage(class std::vector > const &)" (?GetDoubleVectorAverage@CalibrationTool@Technorabble@@YANABV?$vector@NV?$allocator@N@std@@@std@@@Z) referenced in function "private: bool __thiscall MathUtilsTest::GetDoubleVectorAverageTest(void)" (?GetDoubleVectorAverageTest@MathUtilsTest@@AAE_NXZ) C:\_SVN\Technorabble\Windows Software\CalibrationToolUnitTests\MathUtilsTest.obj – technorabble Sep 17 '13 at 15:31
@technorabble: Most likely, you didn't qualify the definition with the correct namespaces after moving it into the source file. Either put it inside the namespaces (as it was in the header), or use the full name `Technorabble::CalibrationTool::GetDoubleVectorAverage`. (And, of course, make sure the source file is being compiled and linked with the program). – Mike Seymour Sep 17 '13 at 15:35
Thanks Mike. I've tried it both of the ways suggested above and am still getting the "unresolved external symbol" linker error. I have CalibrationTool set up as a dependency of CalibrationToolUnitTests, and CalibrationToolUnitTests is last in my build order. Does that satisfy "make sure the source file is being compiled and linked with the program"? – technorabble Sep 17 '13 at 15:48

score 3 · Answer 2 · edited May 23 '17 at 10:25

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.

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

2 Answers2