CODEWITHSUNDEEP
c#
user905481
user905481

Reputation:

cannot return maximum element

I am trying to return maximum element from array using recursion
here is my code

    static void Main(string[] args)
    {
        int[] Array=new int[]{10,233,34};

        int _MaxVal = CalculateMax(Array, 0, 0);
        Console.WriteLine(_MaxVal);
        Console.ReadKey();

    }

    private static int CalculateMax(int[] Array, int Startpos, int maxval)
    {
        if (Startpos != Array.Length)
        {
            if (Array[Startpos] > maxval)
            {
                maxval = Array[Startpos];
            }


            CalculateMax(Array, ++Startpos, maxval);
        }

        return maxval;
    }

I am getting MaxVal as 10 .

What is wrong with it ??

Thanks all

Upvotes: 0

Views: 111

Answers (5)

kokluch
kokluch

Reputation: 602

To use recursion you should have a stop test and be sure to always launch the next step on a smaller problem.

Try this : (Not sure if Array.SubArray(int) is a real function but that is the idea.

static void Main(string[] args)
{
    int[] Array=new int[]{10,233,34};

    int _MaxVal = CalculateMax(Array);
    Console.WriteLine(_MaxVal);
    Console.ReadKey();

}

private static int CalculateMax(int[] Array)
{
    if (Array.Length > 0)
    {
        int maxSubArray = CalculateMax(Array.SubArray(1)); // Use recursive function on the SubArray starting at index 1 (that the smaller problem)
        if (maxSubArray > Array[0])
        {
            return maxSubArray;
        } else {
            return Array[0];
        }

    } else {
        return 0; // Stop test
    }
}

Note : it will not work with negative value.

Upvotes: 1

Egor305
Egor305

Reputation: 49

You don't use return value from deeper calls of CalculateMax

Change

CalculateMax(Array, ++Startpos, maxval);

to

maxval = CalculateMax(Array, ++Startpos, maxval);

This way you pass maxval not only forward, but also backwards to main().

As (probably) said before, recursion is not best way to do it, because stack overflow may happen.

Upvotes: 0

Guffa
Guffa

Reputation: 700302

You are not using the return value from the recursive call. This:

CalculateMax(Array, ++Startpos, maxval);

should be:

maxval = CalculateMax(Array, ++Startpos, maxval);

Anyway, you are using recursion instead of a loop, which is a very bad way to use recursion. You will be doing recursion as deep as there are items in the array, which means that you have a very slow loop, and you will get a stack overflow if the array is too large.

To use recursion correctly you should divide the work into smaller parts, and use recursion to handle those smaller parts, until the parts are so small that they are trivial.

For example, split the array in half for each level:

int[] Array = new int[]{ 10, 233, 34 };
int _MaxVal = CalculateMax(Array, 0, Array.Length);
Console.WriteLine(_MaxVal);

private static int CalculateMax(int[] array, int start, int length) {
  if (length == 1) {
    return array[start];
  } else {
    int half = length / 2;
    int first = CalculateMax(array, start, half);
    int second = CalculateMax(array, start + half, length - half);
    return first > second ? first : second;
  }
}

Upvotes: -1

go..
go..

Reputation: 948

    static void Main(string[] args)
    {
        int[] Array = new int[] { 10, 233, 34 };

        int _MaxVal = CalculateMax(Array);
        Console.WriteLine(_MaxVal);
        Console.ReadKey();

    }
    private static int CalculateMax(int[] Array)
    {
        int max = 0;
        for (int i = 0; i < Array.Length; i++)
            if (Array[i] > max)
                max = Array[i];
        return max;
    }

OR

var max1 = Array.Max();
var max2 = Array.Concat(new[] { 0 }).Max();
var max3 = Array.OrderByDescending(p => p).FirstOrDefault();

Upvotes: 0

Grant Winney
Grant Winney

Reputation: 66449

You're losing the value of maxval.

Try this:

maxval = CalculateMax(Array, ++Startpos, maxval);

Unless you're doing this as a personal exercise or for an assignment, you could handle this way more gracefully with LINQ:

var maxValue = Array.Max();

Upvotes: 7

Related Questions