HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Epilogue to a binary search to find the range of matching indexes

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thesearchrangeindexesbinaryepiloguefindmatching

Problem

I am trying to find out the best practice for writing a loop which does one check and at the same time has to increment a value so that the while loop will eventually fail. Performance is important that's why I thought of using the least amount of variables.

Use case

In a decently low level method I do a binarySearch(array, key) which guarantees to find a key value in a sorted array if the key can be found. However it does not guarantees to find the first or last key first. But since I need the range of indexes in which the key excists (for other code later on) I try to find the last and first element in the array that matches. With this code I am trying to let lastSucceedingIndex hold the last index which holds the key:

while (lastSucceedingIndex < tailIndex && (array[++lastSucceedingIndex] == key)) {
}
lastSucceedingIndex--;


There is one situation in which a bug exists, namely when the lastSucceedingIndex is equal to the tailIndex: the while loop will fail and the ++lastSucceedingIndex will not happen. The lastSucceedingIndex-- is there in the end to fix the ++lastSucceedingIndex when it's not equal to the key, but since this does not happen in the last case it will decrement lastSucceedingIndex incorrectly. I wrote this loop for performance and I take this little bug for granted, it happens so rarely that it will effect less than 0.001% of all cases. This way it seems like I only need one counter and one variable, the loop could be executed as much as 17000 times before it finds its lastSucceedingIndex and the array is very large with possibly up to a million entries.

Are there any alternatives for performance and readability?

I analysed my code by the build in code analysis of my IDE and I get the remark


while statement has empty body at line

Which seems to indicate that having a loop with no lines in them is considered a possible bug by the compiler or at least bad practice. Now my question is, is having an empt

Solution

There are three alternatives you should consider. Depending on circumstances, I have used all three.

The most simple, is to comment the block.

while (lastSucceedingIndex < tailIndex && (array[++lastSucceedingIndex] == key)) {
    //loop condition terminates when key is not found, or data runs out.
}
lastSucceedingIndex--;


The comment should shut down the error.

The second approach is to simplify the loop condition to match your code and to make the dependent code part of the loop. The pre-increment is essentially your "work" which should be in the block... but, because it is a pre-increment, it is messy... Still, your code would become...

while (lastSucceedingIndex < tailIndex) {
    lastSucceedingIndex++;
    if (array[lastSucceedingIndex] != key) {
        break;
    }
}
lastSucceedingIndex--;


That's still not as neat as I would like, but it reduces the complexity of the while condition and makes the break-out process more apparent.

This last alternative is simpler in other ways:

while (lastSucceedingIndex < tailIndex && array[lastSucceedingIndex + 1] == key) {
    lastSucceedingIndex++;
}


Now there is no need for the corrective decrement.

Code Snippets

while (lastSucceedingIndex < tailIndex && (array[++lastSucceedingIndex] == key)) {
    //loop condition terminates when key is not found, or data runs out.
}
lastSucceedingIndex--;
while (lastSucceedingIndex < tailIndex) {
    lastSucceedingIndex++;
    if (array[lastSucceedingIndex] != key) {
        break;
    }
}
lastSucceedingIndex--;
while (lastSucceedingIndex < tailIndex && array[lastSucceedingIndex + 1] == key) {
    lastSucceedingIndex++;
}

Context

StackExchange Code Review Q#90136, answer score: 4

Revisions (0)

No revisions yet.