1
\$\begingroup\$

I would like a review for this simple recursive algorithm which returns the largest int in an array of integers. I would like feedback on the transfer of state via the third parameter specifically. Please also note 2 things:

  1. It must be recursive
  2. This is not for production usage, but rather recursion practice
#include <stdio.h>
#include <stdlib.h>

int max_val(int arr[], int length, int curr_max)
{
    if(length <= 0)
        return curr_max;

    if(curr_max < *arr)
    {
        curr_max = *arr;
    }
    arr++;
    return max_val(arr,length-1,curr_max);
}

int main()
{
    int arr[] = {2,73,39,5,8,62,9};
    printf("Max: %d\n", max_val(arr,7,0));  
    return EXIT_SUCCESS;
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Your code wrongfully prints "Max: 0" for arr[] = {-1, -2, -3} \$\endgroup\$ – Martin R Sep 25 '17 at 5:39
2
\$\begingroup\$

Never omit "optional" braces, as you did for your first if statement. That's a bad habit that will eventually lead to a coding accident. If you feel compelled to omit braces, then at least put the whole statement on the same line.

You should avoid unnecessary mutation, as you did with arr++. This is especially important with an exercise in recursion: it is one of the principles of .

The curr_max parameter is annoying and unnecessary. It also leads to a bug with negative array members, when a poorly selected initial value is chosen. (You should use INT_MIN from <limits.h> instead of 0.) Furthermore, your base case is questionable: is the maximum of a zero-length array well defined?

Suggested solution

#include <assert.h>

int max_val(int arr[], size_t length)
{
    assert(length > 0);
    if (length == 1) return arr[0];
    int max_tail = max_val(arr + 1, length - 1);
    return arr[0] > max_tail ? arr[0] : max_tail;
}
\$\endgroup\$
  • \$\begingroup\$ if (length == 1) return arr[0]; and Never omit "optional" braces do not play well together. \$\endgroup\$ – vnp Sep 25 '17 at 16:43
  • \$\begingroup\$ @vnp I also said that it’s acceptable if you put it all on the same line. \$\endgroup\$ – 200_success Sep 25 '17 at 16:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.