views:

69

answers:

2

Hi,

I implemented a Simple STL map in C++. Factored out the comparison as a type as I was instructed to, then implemented the comparison as shown below:

template <typename T> int KeyCompare<T>::operator () (T tKey1, T tKey2)
{

        if(tKey1 < tKey2)
        return -1;
    else if(tKey1 > tKey2)
        return 1;
    else 
        return 0;
} 

here, tKey1 and tKet2 are the two keys I'm comparing. This worked well for all the basic data types and string. I added a template specialization to compare keys of a user defined type named Test and added a specialization as follows:

int KeyCompare<Test>::operator () (Test tKey1, Test tKey2)
{

        if(tKey1.a < tKey2.a)
        return -1;
    else if(tKey1.a > tKey2.a)
        return 1;
    else 
        return 0;
}

when I run this, I get a linking error saying

SimpleMap.obj : error LNK2005: "public: int __thiscall KeyCompare::operator()(class Test,class Test)" (??R?$KeyCompare@VTest@@@@QAEHVTest@@0@Z) already defined in MapTest.obj

SimpleMap.obj : error LNK2005: "public: __thiscall KeyCompare::~KeyCompare(void)" (??1?$KeyCompare@VTest@@@@QAE@XZ) already defined in MapTest.obj

SimpleMap.obj : error LNK2005: "public: __thiscall KeyCompare::KeyCompare(void)" (??0?$KeyCompare@VTester@@@@QAE@XZ) already defined in MapTest.obj

MapTest.cpp is the test harness class in which I wrote the test case. I have used include guards as well, to stop multiple inclusions.

Any idea what the matter is??

Thank you very much!!

A: 

You don't need a specialisation - simply overload it:

int KeyCompare::operator () (Test tKey1, Test tKey2)
{

        if(tKey1.a < tKey2.a)
        return -1;
    else if(tKey1.a > tKey2.a)
        return 1;
    else 
        return 0;
}

And you should be passing the parameters t all these compare functions as const references.

anon
How an you overload a member function without modifying the class? I don’t think this approach makes much sense …
Konrad Rudolph
@Konrad Who said you couldn't modify the class?
anon
@Neil Butterworth: I say you *shouldn’t*. Make your design extensible, not modifiable. ;-)
Konrad Rudolph
@ Neil: Is it a must that parameters should be const references? If so why?
@isunluck It's not a must (in this case) but if you don't make them const references, non-copyable objects cannot be compared. Remember, for templates you don't know anything about the type that is going to be passed to the template. And using const references for ALL function parameters (except the built-in types) where possible, is simply good C++ practice.
anon
@ Neil: Thanks a lot for the advice :)
+2  A: 

That’s not a specialization.

Also, please show the whole code. You seem to have templatized the operator () method. This is rather unorthodox. Instead, templatize the whole class and specialize it properly.

So instead of your code, write something like this:

template <typename T>
struct KeyCompare {
    int operator ()(T const& key1, T const& key2) const {
        // Comparison logic here …
    }
};

And then specialize the class:

template <>
struct KeyCompare<Test> {
    int operator()(Test const& key1, Test const& key2) const { … }
};

This is slightly more code but makes it truly extensible (since anyone can add their own specialization implementation without having to modify existing code). This is also the way that other C++ libraries (in particular the STL) work.

Konrad Rudolph
yes, I specialized the whole class. I didn't post all the code here, but the class itself does only the comparison of objects, hence it only has this operator () method other than the constructor.
@isurulucky: No, you didn’t specialize the whole class – at least not according to the code you posted. If that code differs from you actual code, then how do you expect us to find the *real* error, as opposed to the error in the simplified code?
Konrad Rudolph
Thanks a lot. I tried what you said and it seems to work :) But can you explain what is happening here? What is the problem when I use the template specialization as I did? I templated the whole class butkept the declaration and definition in two places. But when I use your method, no linker errors..
@isurulucky: Either you omitted the (required) empty `template <>` declaration – your code indicates that – or perhaps you put the implementation into a separate `cpp` file instead of the header. This doesn’t work: As a general rule, templated definitions in C++ must reside in the *header*, not in the implementation file.
Konrad Rudolph
well, I didn't do both. Kept all the code definition and declarations in the header itself and also did not omit the template <>. I did it as follows:template <> class KeyCompare<Tester>{ //specialization code}
@isurulucky: You need to *repeat* `template <>` in front of your method definition if that definition is outside the class.
Konrad Rudolph
Sorry to bother so much, but that is the thing I did first of all, it got me a compile error saying 'KeyCompare<Tester>::{ctor}' : cannot be explicitly specialized.But since now the problem is solved, it is ok. Thank you v. much for the help :)