Loop continuing even after hitting return statement

Question

I have the following method where I wan to keep checking if a nested exception matches an IndexOutOfBoundsException unless the nested exception is the same as the previous one.

It seems to do the right thing where in my test, the first exception is of type NullPointerException thus moving on to the next. The next exception is as expected, an IndexOutOfBoundsException.

When this happens I want to return true which I expect to break me out of the loop. It seems to be happening as expected where I do land on 'return true'. But after that, the loop keeps going.

What am I missing. How is it keeping on going even after returning true?

public boolean test(Throwable throwable) {

    do {
        if(throwable instanceof IndexOutOfBoundsException) {
            return true; // I do land here thus expecting to get out of loop and this method.
        }
        this.test(throwable.getCause());
    } while (throwable.getCause() != throwable);

    return false;
}

The test against it simulating nested exception.

@Test
void testWithNestedException() {
    NullPointerException nullPointerException = new NullPointerException();
    IndexOutOfBoundsException indexOutOfBoundsException = new IndexOutOfBoundsException();
    Throwable nestedException = nullPointerException.initCause(indexOutOfBoundsException);
    assertTrue(someClass.test(nestedException));
}

Maybe you're confused about what 'return' does? It terminates the currently-executing function instance **only**. Since this code is recursive, terminating one level returns to the next level outwards. It does not magically terminate the entire stack of recursive calls. — user13784117, Aug 15 '20 at 11:47
Also, I don't think I've ever encountered a Throwable that is its own cause. `while (throwable.getCause() != throwable);` — user13784117, Aug 15 '20 at 11:50

score 4 · Accepted Answer · answered Aug 15 '20 at 11:44

4

You are mixing recursion with looping. Here, a simple loop that updates the exception being tested should do the trick:

public boolean test(Throwable throwable) {
    Throwable t = throwable;
    do {
        if (throwable instanceof IndexOutOfBoundsException) {
            return true;
        }
        t = t.getCause();
    } while (t.getCause() != t);

    return false;
}

answered Aug 15 '20 at 11:44

Mureinik

297,002
52
306
350

4

When is `t.getCause() != t` ever false? For them to be equal means there's a loop in the exception chain. On the other hand, there's a null pointer exception waiting at the end of the chain. – user13784117 Aug 15 '20 at 11:59
@user13784117 It's actually impossible for a `Throwable` to be its own cause, at least according to the [documentation](https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/lang/Throwable.html#initCause(java.lang.Throwable)) (the `initCause` and `getCause` methods are not final). – Slaw Aug 15 '20 at 13:41
It appears to me that `t = new Throwable(); t.initCause(t);` would work, though I don't advise it. – user13784117 Aug 15 '20 at 13:57
@user13784117 The docs say that would result in an `IllegalArgumentException`. Though again, I suppose someone could override the method(s) and change the behavior to be non-compliant. – Slaw Aug 15 '20 at 14:13

score 2 · Answer 2 · answered Aug 15 '20 at 11:44

2

You are creating recursion with this call and don't use the return code from this call.

this.test(throwable.getCause());

I think you wanted to do:

throwable = throwable.getCause();

answered Aug 15 '20 at 11:44

TomStroemer

1,390
8
28

This is equivalent to @Mureinik's answer, though stylistically I prefer his introduction of a local variable. – user13784117 Aug 15 '20 at 12:09
Yeah, i was 10 seconds quicker ;-) – TomStroemer Aug 15 '20 at 12:55

Stephen C · Answer 3 · 2020-08-15T12:04:47.677

As @Mureinik points out, you are mixing recursion and iteration and doing neither correctly. A (correct) recursive version would be:

public boolean test(Throwable throwable) {
    if (throwable == null) {
        return false;
    } else if (throwable instanceof IndexOutOfBoundsException) {
        return true;
    } else {
        return test(throwable.getCause());
    }
}

In my opinion, a recursive version is easier to understand than an iterative one, but others may disagree.

With the recursive version, there is the theoretical problem that sufficiently deeply nested exceptions could lead to a stack overflow. Getting that to happen in practice would require some rather contrived (i.e. unrealistic) code, so it should be safe to ignore this.

Not only is this code correct, it's now clear to see what it does: determines whether any exception in the chain is "index out of bounds". Clarity is as important as correctness, IMO. — user13784117, Aug 15 '20 at 11:55

Loop continuing even after hitting return statement

3 Answers3