Logical error in Function template

Question

My professor has given me this assignment.

Implement a generic function called Max, which takes 3 arguments of generic type and returns maximum out of these 3. Implement a specialized function for char* types.

Here's my code :

#include <iostream>
#include <string>

using namespace std;

template<typename T>
T Max(T first,T second,T third )
{
    if(first > second)
    {
        if(first > third)
        {
            return first;
        }
        else
        {
            return third;
        }
    }
    else if(second > third)
    {
        return second;
    }
    else
    {
        return third;
    }
}


template<>
char* Max(char* first,char* second,char* third)
{   
    if(strcmp(first, second) > 0)
    {
        if(strcmp(first, third) > 0)
        {
            return first;
        }
        else
        {
            return third;
        }
    }
    else if(strcmp(second, third) > 0)
    {
        return second;
    }
    else
    {
        return third;
    }
}

int main(void)
{
    cout << "Greatest in 10, 20, 30 is " << Max(10, 20, 30) << endl;

    char a = 'A';
    char b = 'B';
    char c = 'C';
    char Cptr = *Max(&a, &b, &c);
    cout << "Greatest in A, B ,C is " << Cptr << endl;

    string d = "A";
    string e = "B";
    string f = "C";
    string result = *Max(&d, &e, &f);

    cout << "Greatest in A, B, C is " << result << endl;
}

Output :

Greatest in 10, 20, 30 is 30
Greatest in A, B ,C is C
Greatest in A, B, C is A

Problem :

If I pass char datatypes in Max function A, B, C, it returns C, but if I pass string datatypes A, B, C it returns A.

Why does it return A here?

Answer 1

There are two problems here. The other two answers have already described the problem with your third call.

But your second call is also wrong:

char a = 'A';
char b = 'B';
char c = 'C';
char Cptr = *Max(&a, &b, &c);

This should produce undefined behaviour since strcmp expects zero-terminated strings but this isn’t what you feed into the function. Rather, you are passing it pointers to individual char values and strcmp has every right to choke on this. Basically, anything can happen and that your code works is pure chance.

The right way to call this overload would be just to pass chars, or to pass C-style strings:

char C = Max(a, b, c);

// or:
char as[] = "a";
char bs[] = "b";
char cd[] = "c";
char* result = Max(as, bs, cd);

Alternatively, you could pass the string literals directly.

Finally, a note on style. You char* specialization can be shortened considerably if you “cheat” a little bit by converting the incoming char* strings to proper std::strings and re-using the generic version of Max:

template<>
char* Max(char* first,char* second,char* third)
{
    return Max(string(first), string(second), string(third));
}

(Granted, this probably is less efficient but in most cases this can be safely ignored.)

And yet another remark: the assignment explicitly asked you to specialize the function template for char* so your answer is correct. However, an alternative would be to overload the function instead of specializing it. For function templates (as opposed to class templates), this is the usual way when you don’t require more template arguments:

char* Max(char* first,char* second,char* third)
{
    return Max(string(first), string(second), string(third));
}

Note that the only difference is the missing template <> in front of the function header.

Answer 2

In the first case, it uses the template specialization, in the second case, the generic template.

But your problem is the way you call Max in the second case:

string d = "A";
string e = "B";
string f = "C";
// you're comparing the string addresses here, not their content
string result = *Max(&d, &e, &f); 

Should be:

string d = "A";
string e = "B";
string f = "C";
string result = Max(d, e, f);

Also, I would suggest using const pointers in the char* specialization because as it stands, you cannot pass anything but non-const pointers which is not exactly the common case.

Answer 3

string result = *Max(&d, &e, &f);

This line is your issue. You are passing the pointer to the string in so it is in fact returning the highest pointer address.

Bear in mind that the stack grows downwards so the start of the stack has the highest address. Each subsequent stack allocation (ie variable declaration in this case) will start a progressively lower stack address and hence "A" shows up as the largest value.

If you write this:

string result = Max(d, e, f);

You'll get the answer you expect.

Answer 4

instead of

string result = *Max(*&d, &e, &f)

you need

string result = Max(d.c_str(), e.c_str(), f.c_str())

Note that this will work if your function takes const char* and not char*. If you insist on using char*, which is wrong, then you'll have to cast away constness

 string result = Max(const_cast<char*>(d.c_str()),

 const_cast<char*>(e.c_str()), const_cast<char*>(f.c_str()));

but since you are using strings, note that you can compare them simply with == < > etc.