This Question is from Hacker rank Function in C section
I am getting the answer that i want but the output is repeated so many time that i have to stop the code from running manually

#include <iostream>
    #include <cstdio>
    using namespace std;
    
    int max_of_four(int a,int b,int c,int d){
        if (a>b){
            cout<<a;
        }else if(b>c){
            cout<<b;
        }else if(c>d){
            cout<<c;
        }else if(d>c){
            cout<<d;
        }
        return max_of_four( a,  b,  c,  d);
    }
    
    int main() {
        int a, b, c, d;
        scanf("%d %d %d %d", &a, &b, &c, &d);
        int ans = max_of_four(a, b, c, d);
        printf("%d", ans);
        
        return 0;
    }

CodePudding user response：

The problem is that you have written max_of_four() as a recursive function. It calls itself over and over in an endless loop, and it is printing on each iteration. The function should not be printing anything at all (the caller is printing the value that is returned), and it certainly should not be calling itself at all.

Also, the logic is just plain wrong. It doesn't actually report the largest value of the 4 values given. For instance, if a>b is true then it doesn't even consider the values of c and d at all, which might be larger than a. And similarly, if b>c is true then the value of d is not considered.

The correct logic would look more like this:

int max_of_four(int a,int b,int c,int d){
    int max_value = a;
    if (b > max_value){
        max_value = b;
    }
    if (c > max_value){
        max_value = c;
    }
    if (d > max_value){
        max_value = d;
    }
    return max_value;
}

Which can be simplified using the std::max() algorithm, eg:

#include <algorithm>

int max_of_four(int a,int b,int c,int d){
    return std::max(a, std::max(b, std::max(c, d)));
    // Or: return std::max({a,b,c,d});
}

Alternatively, using the standard std::max_element() algorithm, eg:

#include <algorithm>

int max_of_four(int a,int b,int c,int d){
    int arr[] = {a,b,c,d};
    return *std::max_element(arr, arr 4);
}

CodePudding user response：

You are calling same-method inside itself (wihtout any break).

The code should be:

int max_of_four(int a, int b, int c, int d)
{
    if (a > b) {
        cout << a;
        return a;
    } else if (b > c) {
        cout << b;
        return b;
    } else if (c > d) {
        cout << c;
        return c;
    } else if (d > c) {
        cout << d;
        return d;
    }
}

CodePudding user response：

The problem lies in the recursive call you are making inside of max_of_four(). What you should do is replace the cout (which prints the result) with return statements. Edit the code to look like this instead:

int max_of_four(int a,int b,int c,int d){
    if (a>b){
        return a;
    }else if(b>c){
        return b;
    }else if(c>d){
        return c;
    }else if(d>c){
        return d;
    }
}

There is a logical problem as well, this code wouldn't work if, for example, d is the greatest number but a>b is true. But that's a separate issue. At least this is the explanation why you get the same answer over and over again.