Else is simply skipped, but only sometimes-CodePudding

private int bkicalc(int i, List<Integer> history) {

    List<Integer> subList = history.subList(history.size()-1 - i, history.size()-1);
    Set<Integer> uniliste = new HashSet<Integer>(subList);
    
    if (uniliste.size() == 4) {
        if (i>5) {
            System.out.println(i);
        }
        return i;
    } else {
        this.bkicalc(i   1, history);
    }
    
       System.out.println("else was skipped");
    return 404;

}

Hi, this method is accessed a 1000 times from a for-loop, and in 1/10 of times it skips the else and returns the 404. I cannot explain to myself why this is happening.

Hope you can help me out.

CodePudding user response：

You return from the if branch, but not from the else branch. Therefore, the diagnostic and 404 return do not indicate that the else was skipped, but rather that it was taken. That the else branch recurses is irrelevant: unless an exception is thrown from the recursive invocation, that invocation will eventually return, at which point control simply passes out of the else block.

It looks like you probably want in the else branch to return the result of the recursive invocation:

private int bkicalc(int i, List<Integer> history) {

    List<Integer> subList = history.subList(history.size()-1 - i, history.size()-1);
    Set<Integer> uniliste = new HashSet<Integer>(subList);
    
    if (uniliste.size() == 4) {
        if (i>5) {
            System.out.println(i);
        }
        return i;
    } else {
        return this.bkicalc(i   1, history);  // <-- here
    }
    
    assert false : "unreachable";
    return 404;
}

You might even find with that that the compiler actually flags the return 404 as unreachable.

Note also that the bounds on your sublist are a bit surprising. The upper bound provided to List.subList() is exclusive, not inclusive, so you are consistently omitting the the last element of history from consideration. If that's intentional then it would be wise to add a code comment saying so.

Furthermore, if history does not in fact contain at least 4 distinct elements overall, then after some number of recursions, this method will fail with an IndexOutOfBoundsException. This is of course certain to happen if it does not initially contain at least 5 elements total (remembering that one is ignored).

Finally, the recursion seems wasteful and unnecessarily confusing. An iterative approach would be better, for a variety of reasons. (It almost always is.) Example:

private int bkicalc(int i, List<Integer> history) {
    if (history.size() >= 5) {
        Set<Integer> uniliste = new HashSet<Integer>(
                history.subList(history.size() - 5, history.size() - 1));
        ListIterator<Integer> iterator = history.listIterator(history.size() - 5);

        while (uniliste.size() < 4 && iterator.hasPrevious()) {
            uniliste.add(iterator.previous());
        }

        if (uniliste.size() == 4) {
            int i = history.size() - 1 - iterator.nextIndex();

            if (i > 5) {
                System.out.println(i);
            }
            return i;
        }
    }

    // Fewer than 4 distinct elements (ignoring the last)
    return -1;
}