5

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;
}
2
  • No that you've had some feed back... which option were you backing ? Oct 29, 2008 at 15:27
  • I wrote option B. One of our applicants (we're currently recruiting) came up with option A, and told me I "... should ask some independent programmer to compare two approaches and tell which one is better..." Thanks to everyone for helping me justify my response to him!
    – user32400
    Oct 29, 2008 at 15:35

8 Answers 8

15

Ô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)

0
10

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).

4
  • I hadn't thought of this, as I was immediately distracted by the O() complexity differences, but a very valid observation. Oct 29, 2008 at 15:41
  • The method in question is part of a small programming test (we're currently recruiting), and modifying the input array is one of the criteria we use to fail the test.
    – user32400
    Oct 29, 2008 at 15:48
  • Sure, when can you start? ;o)
    – user32400
    Oct 29, 2008 at 16:36
  • By the way, I agree that -1 is a lousy error return value for this method, but that's how the test is worded.
    – user32400
    Oct 29, 2008 at 16:37
5

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.

1
  • If you could add a comment about the exception handling for non-exceptional cases in option A, I'd be compelled to vote for your answer! Oct 29, 2008 at 15:18
5

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

2

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

0

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.

1
  • see olavk's answer about the sorting side effect
    – Karg
    Oct 29, 2008 at 15:38
0

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

0

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.