Fix calculation for slot containing offset

[mattwar]

Fixes issue #605

Add logic to not return zero-length nodes, since they never contain their offset.

[mattwar]

@pharring might want to comment.

[pharring]

We need the same fix for VB.

[pharring]

Yeah, I wondered about this case when I wrote it. I originally had a hand-coded binary search that avoided this problem, but I was 'encouraged' to change it to Array.BinarySearch in review.

What we really need is an "upper_bound" equivalent (Array.BinarySearch gives the lower bound).

Note that this BinarySearch is already an extension method over int[]. We could introduce a "UpperBoundBinarySearch" method. That would be nice to avoid code duplication (both C# and VB need the same fix).

[pharring]

Here is an "upper_bound" binary search algorithm. The difference from Array.BinarySearch and the existing BinarySearch extension method is subtle, but significant: We don't check for equality in the middle. We keep halving the search space until there's only a single value. Note that, since it's an upper bound, it can return a value equal to array.Length (i.e. off the end of the array).

/// <summary>
/// Search a sorted integer array for the target value in O(log N) time.
/// </summary>
/// <param name="array">The array of integers which must be sorted in ascending order.</param>
/// <param name="value">The target value.</param>
/// <returns>An index in the array pointing to the position where <paramref name="value"/> should be
/// inserted in order to maintain the sorted order. All values to the right of this position will be
/// strictly greater than <paramref name="value"/>. Note that this may return a position off the end
/// of the array if all elements are less than or equal to <paramref name="value"/>.</returns>
static int BinarySearchUpperBound(this int[] array, int value)
{
    int low = 0;
    int high = array.Length - 1;

    while (low <= high)
    {
        int middle = low + ((high - low) >> 1);
        if (array[middle] > value)
        {
            high = middle - 1;
        }
        else
        {
            low = middle + 1;
        }
    }

    return low;
}

With this, I believe FindSlotIndexContainingOffset can be implemented like this:

return _childOffsets.BinarySearchUpperBound(offset) - 1;

[pharring]

@mattwar Let me know if you want me to take care of this for you. (I feel bad since I introduced the bug in the first place)

[mattwar]

@pharring please review the newest changes

[pharring]

👍 please merge at your earliest convenience.