views:

57

answers:

4

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?

Thanks.

+2  A: 
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.

Goz
@Goz Thanks : )
Searock
+2  A: 

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.

ereOn
@ereOn thanks for your help.
Searock
+1  A: 

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.

Armen Tsirunyan
what does `c_str()` do ?
Searock
This wouldn't work: the template specialization takes only `char*` not `const char*`. Moreover it would invoke unnecessary conversions to C-String.
ereOn
c_str converts from string to const char*
Armen Tsirunyan
@Searock: `c_str()` gives a NULL-terminated `const char*` (a C-String).
ereOn
@Armen Tsirunyan @ereOn Thanks.
Searock
Armen Tsirunyan
@Armen: `const_cast` on the result of `c_str` will yield undefined behaviour as far as I understand the standard (§21.3.6. doesn’t specify the type of storage used for the return value of `c_str`).
Konrad Rudolph
Using char* is not "wrong" as it was what he was asked to do, not get it to work with std::string. There is really no need to specialise it to work with std::string as the default version of the template handles that case. The Max function overload for char* is supposed to also return a writable buffer so in itself is not an issue. I would test it with several cases with different orderings, so "One", "Two", "Three" (order 2>3>1) "January", "February" "March" (3>1>2) "Monday", "Tuesday", "Wednesday" (3>2>1), actually 6 examples with the different orderings would be best.
CashCow
@CashCow: Using char* is in this case WRONG, on the other hand, using const char* is not. That was my point
Armen Tsirunyan
Using char* would be correct if you want to end up with a buffer you can write into.
CashCow
@CashCow: in this particular case, where the function finds the maximum of 3 strings, provided that the user is willing to use c-strings, and nothing else, passing parameters as const char * is correct, and passing them as char* is incorrect. period.
Armen Tsirunyan
+4  A: 

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.

Konrad Rudolph
@Konrad Rudolph thanks.
Searock