Which of these two GetLargestValue C# implementations is better, and why?

Question

I'm having a disagreement with someone over how best to implement a simple method that takes an array of integers, and returns the highest integer (using C# 2.0).

Below are the two implementations - I have my own opinion of which is better, and why, but I'd appreciate any impartial opinions.

Option A

public int GetLargestValue(int[] values)
{
   try  {
          Array.Sort(values);
          return values[values.Length - 1];
        }
   catch (Exception){ return -1;}
}

Option B

public int GetLargestValue(int[] values)
{
    if(values == null)
        return -1;

    if(values.Length < 1)
        return -1;

    int highestValue = values[0];

    foreach(int value in values)
        if(value > highestValue)
            highestValue = value;

    return highestValue;
}

Answer 1

I prefer Option B as it only traverses the collection exactly once.

In Option A, you may have to access many elements more than once (the number of times is dependant upon the implementation of the sort alogrithm).

The Option A is an inefficent implementation, but results in a fairly clear algorithm. It does however use a fairly ugly Exception catch which would only ever be triggered if an empty array is passed in (so could probably be written clearer with a pre-sort check).

PS, you should never simply catch "Exception" and then correct things. There are many types of Exceptions and generally you should catch each possible one and handle accordingly.

Answer 2

The second one is better. The complexity of the first ` O(N LogN) ..for the second, its O(N)

Kind regards,

Answer 3

I have to choose option B - not that it's perfect but because option A uses exceptions to represent logic.

Answer 4

Ôption B of course.

A is ugly :

• Catch(Exception) is a really bad practice

• You shoul not rely on exception for null ref, out of range,...

• Sorting is way complexier than iteration

Complexity :

• A will be O(n log(n)) and even O(n²) in worst case

• B worst case is O(n)

Answer 5

I would say that it depends on what your goal is, speed or readability.

If processing speed is your goal, I'd say the second solution, but if readability is the goal, I'd pick the first one.

I'd probably go for speed for this type of function, so I'd pick the second one.

Answer 6

There are many factors to consider here. Both options should include the bounds checks that are in option B and do away with using Exception handling in that manner. The second option should perform better most of the time as it only needs to traverse the array once. However, if the data was already sorted or needed to be sorted; then Option A would be preferable.

No sorting algorithm performs in n time, so Option B will be the fastest on average.

Edit: Article on sorting

Answer 7

I see two points here:

1. Parameter testing as opposed to exception handling: Better use explicit checking, should also be faster.
2. Sorting and picking the largest value as opposed to walking the whole array. Since sorting involves handling each integer in the array at least once, it will not perform as well as walking the whole array (only) once.

Which is better? For the first point, definitely explicit checking. For the second, it depends...

The first example is shorter, makes it quicker to write and read/understand. The second is faster. So: If runtime efficiency is an issue, choose the second option. If fast coding is your goal, use the first one.

Answer 8

A has the side effect that it sorts the array. This might be unexpected by the caller.

Edit: I don't like to return -1 for empty or null array (in both solutions), since -1 might be a legal value in the array. This should really generate an exception (perhaps ArgumentException).