views:

178

answers:

3

Here is a small code-example from which I'd like to ask a question :

complex.h :

#ifndef COMPLEX_H
#define COMPLEX_H

#include <iostream>

class Complex
{
public:
   Complex(float Real, float Imaginary);

   float real() const { return m_Real; };

private:
   friend std::ostream& operator<<(std::ostream& o, const Complex& Cplx);

   float m_Real;
   float m_Imaginary;
};

std::ostream& operator<<(std::ostream& o, const Complex& Cplx) {
   return o << Cplx.m_Real << " i" << Cplx.m_Imaginary;
}
#endif // COMPLEX_H

complex.cpp :

#include "complex.h"

Complex::Complex(float Real, float Imaginary) {
   m_Real = Real;
   m_Imaginary = Imaginary;
}

main.cpp :

#include "complex.h"
#include <iostream>

int main()
{
   Complex Foo(3.4, 4.5);
   std::cout << Foo << "\n";
   return 0;
}

When compiling this code, I get the following error :

multiple definition of operator<<(std::ostream&, Complex const&)

I've found that making this fonction inline solves the problem, but I don't understand why. Why does the compiler complain about multiple definition ? My header file is guarded (with #define COMPLEX_H).

And, if complaining about the operator<< fonction, why not complain about the public real() fonction, which is defined in the header as well ?

And is there another solution as using the inline keyword ?

+4  A: 

Move implementation to complex.cpp

Right now after including this file implementation is being compiled to every file. Later during linking there's a obvious conflict because of duplicate implementations.

::real() is not reported because it's inline implicitly (implementation inside class definition)

XAder
+5  A: 

The problem is that the following piece of code is a definition, not a declaration:

std::ostream& operator<<(std::ostream& o, const Complex& Cplx) {
   return o << Cplx.m_Real << " i" << Cplx.m_Imaginary;
}

You can either mark the function above and make it "inline" so that multiple translation units may define it:

inline std::ostream& operator<<(std::ostream& o, const Complex& Cplx) {
   return o << Cplx.m_Real << " i" << Cplx.m_Imaginary;
}

Or you can simply move the original definition of the function to the "complex.cpp" source file.

The compiler does not complain about "real()" because it is implicitly inlined (any member function whose body is given in the class declaration is interpreted as if it had been declared "inline"). The preprocessor guards prevent your header from being included more than once from a single translation unit ("*.cpp" source file"). However, both translation units see the same header file. Basically, the compiler compiles "main.cpp" to "main.o" (including any definitions given in the headers included by "main.cpp"), and the compiler separately compiles "complex.cpp" to "complex.o" (including any definitions given in the headers included by "complex.cpp"). Then the linker merges "main.o" and "complex.o" into a single binary file; it is at this point that the linker finds two definitions for a function of the same name. It is also at this point that the linker attempts to resolve external references (e.g. "main.o" refers to "Complex::Complex" but does not have a definition for that function... the linker locates the definition from "complex.o", and resolves that reference).

Michael Aaron Safyan
A: 

And is there another solution as using the inline keyword?

Yes, there is. Apart form defining the method inside the implementation file complex.cpp as mentioned by others, you can also put the definition into a nameless namespace.

namespace {
    std::ostream& operator<<(std::ostream& o, const Complex& Cplx) {
        return o << Cplx.m_Real << " i" << Cplx.m_Imaginary;
    }
}

In practice, this will create a unique namespace for each compilation unit. That way, you prevent the name clashes. However, the names are still exported from the compilation unit but useless (since the names are unknown).

Putting the definition inside the implementation file is often a better solution. However, for class templates you can’t do that since C++ compilers don’t support instantiating templates in a compilation unit different from the one they were defined in. In those case, you have to use either inline or an unnamed namespace.

Konrad Rudolph
This has the severe disadvantage that every translation unit including this header gets its own copy of the implementation. Is there anything I'm missing or is this really as stupid as it seems to me?
sbi
@sbi: the same is of course true for `inline` (and that’s no big deal). So yes, you’re missing something. – For simple classes, putting everything inside the implementation file is usually better. But for templates you don’t have the choice (see updated answer).
Konrad Rudolph
But with `inline` I at least (hopefully, I know) have the advantage of not having a function call when invoking the code. With your example I see no advantage, except for the fact that a cpp file isn't needed. And that would be taken care of by `inline` just fine.
sbi
@sbi: The question was: “is there an alternative” and my answer is yes, there is. I don’t understand your objections. Furthermore, what makes you think that calls in an unnamed namespace won’t get inlined? It’s not as if modern optimizing compilers care about `inline` all that much to decide which calls to inline.
Konrad Rudolph
Yes, this is indeed a syntactically possible alternative, but I still don't like it for the reasons given. So I down-voted your answer and, being a good citizen, I explained my vote. You might not like my objections. But then I don't like your alternative either.
sbi
BTW, I don't understand your comment to http://stackoverflow.com/questions/2727890/2727905#2727905. (And I can't comment either, since you deleted the answer.) TTBOMK, this `bool operator() const (Book lhs, Book rhs)` is invalid syntax. I pointed it out, and since it looked like a brainfart to me, I tried some humor. I'm sorry if that went wrong.
sbi
@sbi: I was just too dump to realize the syntax was wrong, and I deleted because the answer was then redundant. Anyway, thanks for pointing that out, even if I didn’t get it.
Konrad Rudolph
Ok. Thanks for explaining. I was a bit confused there. `:)`
sbi
Inlining can also be a disadvantage depending on the size of the function. So I think, Konrad really provides a sensible alternative to inline here.
Bernhard Kausler