Binary search stops working when more than 44 elements in an array

Question

This is my first time posting here, so forgive stupid mistakes if such happen.

I am a beginner in C coding (learning right now), and frankly not much of a math genius so the problem I encountered stumps me quite a bit. I found a solution to it by the way of testing different things, but honestly I don't exactly understand why my solution does work, and the code before the solution applied doesn't.

The code is a binary search for a given number in a sorted array created by using seeded srand48, and sorted with a linear sort.

To frame the situation. Code before the solution did work well until array reached 45 elements, then it stopped finding the number.

Before the solution code:

bool search(int value, int values[], int n)
{
    int first, middle, last;

    first = 0;
    last = n - 1;
    middle = (first + last) / 2;

    if (n > 0)
    {
        while (first < last)
        {
            if (values[middle] == value)
            {
                return true;
            }
            else if (values[middle] < value)
            {
                first = middle + 1;
            }
            else
            {
                last = middle - 1;
            }
            middle = (first + last) / 2;
        }
    }
    else
    {
        return false;
    }
    return 0;
}

The solution I found was just removing + and - 1 from the new middle value assignment. After I did this it works perfectly fine. What I would like to know for my own future reference and learning is why exactly the first code does not work, and why did that solution fix it. (I might somehow subconsciously understand how this works and that is why I came up with the fix, but I can't consciously figure out the logic behind it.)

Answer 1

There are some subtle problems in your code:

• It does not work if the array has a single element: it will always return 0. This problem is actually causing your observed behavior: you test while (first < last) but first and last are both included in the range to search. You should either use while (first <= last) or change the algorithm to exclude last, which I favor, as you no longer need to special case the empty array with this solution.

• It can return false or 0 in case of failure, which is OK in C but different in javascript. You should remove the else clause.

• The computation for middle has a potential arithmetic overflow if n is larger than half the maximum value if int and the value is large enough. This is actually a classic bug that remained in hiding for many years in many standard C libraries. Instead of middle = (first + last) / 2, you should use:

  middle = first + (last - first) / 2;
  

Here is a corrected and simplified version:

bool search(int value, int values[], int n) {
    int first = 0, last = n;

    while (first < last) {
        int middle = first + (last - first) / 2;
        if (values[middle] == value) {
            return true;
        }
        if (values[middle] < value) {
            first = middle + 1;
        } else {
            last = middle;
        }
    }
    return false;
}

Note that the middle value where value was found is not necessarily the smallest index where is appears if there are duplicates in the array. It is not a problem here as you are only interested in the boolean result, but the algorithm must altered if you want to find the smallest index.

Matt Timmermans posted a better algorithm in another question that is both more efficient and has this property:

bool search(int value, int values[], int n) {
    int pos = 0;
    int limit = n;

    while (pos < limit) {
        int middle = pos + ((limit - pos) >> 1);

        if (values[middle] < value)
            pos = middle + 1;
        else
            limit = middle;
    }
    return pos < n && values[pos] == value;
}

As to your final question: why does removing the + 1 and -1 from your code fix the problem? Your fix does not fully correct the problem: your changes have this effect:

• first = middle; is just suboptimal, the search will take marginally longer but no other effect.
• last = middle; fixes the problem because the loop assumes last to be excluded, which is consistent with last = middle;.
• The remaining problem is the initial value for last, which should be last = n;.
• With last = n - 1;, you will still fail to locate the value if it is at the end of the array without duplicates, including if the array has only one element.