Question

The following code should return he closest value to a input number 'x' in the sorted array. The while loop is guaranteed to return. But the compiler understandably forces me to return a value. My question is about best practice involved. What is the general convention used such cases where return statement is forced by compiler ? ie throw exception ? return default ? etc ?

NOTE: I have selected "IllegalArgumentException" http://codereview.stackexchange.com/questions/47328/find-the-value-nearest-to-k-in-the-sorted-array. I dont have any answer to select, but Thank all for insight. I am not aware about SO protocols in such cases

/**
 * Given a sorted array returns the 'closest' element to the input 'x'.
 * 'closest' is defined as Math.min(Math.abs(x), Math.abs(y)) 
 * 
 * Expects a sorted array, and if array is not sorted then results are unpredictable.
 * 
 * @param a  The sorted array a
 * @return   The nearest element
 */
public static int getClosestK(int[] a, int x) {

    int low = 0;
    int high = a.length - 1;

    while (low <= high) {
        int mid = (low + high) / 2;

        // test lower case
        if (mid == 0) {
            if (a.length == 1) {
                return a[0];
            }

            return Math.min(Math.abs(a[0] - x), Math.abs(a[1] - x)) + x;
        }


        // test upper case
        if (mid == a.length - 1) {
            return a[a.length - 1];
        }

        // test equality
        if (a[mid] == x || a[mid + 1] == x) {
            return x;
        }


        // test perfect range.
        if (a[mid] < x  && a[mid + 1] > x) {
            return Math.min(Math.abs(a[mid] - x), Math.abs(a[mid + 1] - x)) + x;
        }

        // keep searching.
        if (a[mid] < x) {
            low = mid + 1;
        } else { 
            high = mid;
        }
    }

    //  return a[0]; ?????
}
Was it helpful?

Solution 2

Compiler is forcing you to return the value because it may be possible that you never satisfy the condition for the while loop, hence you don't enter the while loop and return the value.

In that case you won't be having return statement for the method which is supposed to return an int.

In your case if length of int[] a is 0, you wont be returning anything as you don't satisfy condition for while loop.

In such scenario, you need to validate the input and if input does not pass validation check, you can return an exception. Here, IllegalArgumentException seems suitable to me, but it totally depends on you that which exception you want to throw.

throw new IllegalArgumentException("Error message");

OTHER TIPS

If you know that a value will be returned nevertheless but the compiler cannot determine it, this is a programmer error therefore you can allow yourself to::

throw new IllegalStateException();

instead of returning a value which would be wrong anyway.

What about when a is an empty array? Then the following will happen:

public static int getClosestK(int[] a, int x) {
    int low = 0;
    int high = a.length - 1; // 0 - 1 = -1

    while (low <= high) { // 0 <= -1 -> false
        // ...
    }
    // There is no return here. What should happen?
}

That's why the compiler forces you to return.

If you are sure that the cycle can not end normally, you can just replace the cycle condition with the plain while( true ) without changing the code semantics.

In general, if you are sure that something will not happen, throw an AssertionError at that point with the message explaining the "impossible condition". In your case it could be something like

throw new AssertionError("not reachable");

AssertionError is better suited to mark "impossible" conditions, while IllegalStateException is the proper exception for situations when the system state does not allow to perform some otherwise valid operation.

The issue you have is that your while loop guarantee the return but it is possible that will never be entered.

As your initialization values are

int low  = 0;
int high = a.length - 1;

in case when you pass an empty array you will get

while(0 <= -1) {
 ...
}

As @fge, suggested you may want to throw an exception about Illegal State.

You can throw a RuntimeException, it is the exemple which is take from the Java Language Specification : this

If a method is declared to have a return type, then every return statement (§14.17) in its body must have an Expression. A compile-time error occurs if the body of the method can complete normally (§14.1). In other words, a method with a return type must return only by using a return statement that provides a value return; it is not allowed to "drop off the end of its body." Note that it is possible for a method to have a declared return type and yet contain no return statements. Here is one example:

     class DizzyDean {
        int pitch() { throw new RuntimeException("90 mph?!"); }
    }

You have two options:

  • Return some "dummy" value
  • Throw exception

I will suggest you to use exception (some runtime exception - IllegalStateException probably will be good one), since return "error" value looks more like C-style coding.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top