Welcome to ShenZhenJia Knowledge Sharing Community for programmer and developer-Open, Learning and Share
menu search
person
Welcome To Ask or Share your Answers For Others

Categories

I have an array

Values array: 12 20 32 40 52
              ^  ^  ^  ^  ^
              0  1  2  3  4

on which I have to perform binary search to find the index of the range in which the number lies. For example:

  1. Given the number -> 19 (It lies between index 0 and 1), return 0
  2. Given the number -> 22 (It lies between index 1 and 2), return 1
  3. Given the number -> 40 (It lies between index 3 and 4), return 3

I implemented the binary search in the following manner, and this comes to be correct for case 1, and 3 but incorrect if we search for case 2 or 52, 55 32, etc.

#include <iostream>
using namespace std;

int findIndex(int values[], int number, unsigned first, unsigned last)
{
    unsigned midPoint;
    while(first<last)
    {
        unsigned midPoint = (first+last)/2;
        if (number <= values[midPoint])
            last = midPoint -1;
        else if (number > values[midPoint])
            first = midPoint + 1;
    }
    return midPoint;
}


int main()
{
    int a[] = {12, 20, 32, 40, 52};
    unsigned i = findIndex(a, 55, 0, 4);
    cout << i;
}

Use of additional variables such as bool found is not allowed.

See Question&Answers more detail:os

与恶龙缠斗过久,自身亦成为恶龙;凝视深渊过久,深渊将回以凝视…
thumb_up_alt 0 like thumb_down_alt 0 dislike
612 views
Welcome To Ask or Share your Answers For Others

1 Answer

A range in C or C++ is normally given as the pointing directly to the lower bound, but one past the upper bound. Unless you're feeling extremely masochistic, you probably want to stick to that convention in your search as well.

Assuming you're going to follow that, your last = midpoint-1; is incorrect. Rather, you want to set last to one past the end of the range you're going to actually use, so it should be last = midpoint;

You also only really need one comparison, not two. In a binary search as long as the two bounds aren't equal, you're going to set either the lower or the upper bound to the center point, so you only need to do one comparison to decide which.

At least by convention, in C++, you do all your comparisons using < instead of <=, >, etc. Any of the above can work, but following the convention of using only < keeps from imposing extra (unnecessary) requirements on contained types.

Though most interviewers probably don't care, there's also a potential overflow when you do midpoint = (left + right)/2;. I'd generally prefer midpoint = left + (right - left)/2;

Taking those into account, code might look something like this:

template <class T>
T *lower_bound(T *left, T *right, T val) {
    while (left < right) {
        T *middle = left + (right - left) / 2;
        if (*middle < val)
            left = middle + 1;
        else
            right = middle;
    }
    return left;
}

template <class T>
T *upper_bound(T *left, T *right, T val) {
    while (left < right) {
        T *middle = left + (right - left) / 2;
        if (val < *middle)
            right = middle;
        else
            left = middle + 1;
    }
    return left;
}

与恶龙缠斗过久,自身亦成为恶龙;凝视深渊过久,深渊将回以凝视…
thumb_up_alt 0 like thumb_down_alt 0 dislike
Welcome to ShenZhenJia Knowledge Sharing Community for programmer and developer-Open, Learning and Share
...