Binary search implementation

Started by
12 comments, last by Waterlimon 10 years ago

I did not consult any reference for this implementation, so I guess there are bugs lurking everywhere.

Constructive criticisms welcome. And yes, it is C-ish.


 
int BinarySearch(int data[], unsigned int elements, int key)
{
    if(elements==0)
    {
        cerr << "Binary search not able to perform on 0 element array";
        exit(-1);
    }
    unsigned int half_offset = elements<<31? (elements-1)/2 : elements/2;   // truncation will happen anyway.
    unsigned int current_index = elements<<31? (elements-1)/2 : elements/2; // Non portable bitwise ops.
    
        
    while(half_offset!=0)
    {
        if(data[current_index]==key)
            return current_index;
        else if(data[current_index]<key)
        {
            current_index += half_offset<<31? (half_offset+1)/2 : half_offset/2;
        }
        else if(data[current_index]>key)
        {
            current_index -= half_offset<<31? (half_offset+1)/2 : half_offset/2;
        }
        half_offset /= 2; // Truncated to floor
    }
    if(data[current_index]==key)
                return current_index;
}
Intel Core 2 Quad CPU Q6600, 2.4 GHz. 3GB RAM. ATI Radeon HD 3400.
Advertisement
There's no need to check if a number is odd prior to do integer division: 10/2 == 11/2 == 5.
There also isn't any need to check if a number is odd to do integer division, rounding up: (12+1)/2 == (11+1)/2 == 6.
Shifting by 31 bits to determine if the first bit is set has the potential of breaking, if unsigned int isn't 32-bits. It is more idiomatic to check if an integer is odd by testing its first bit: elements&1 ? odd : even.

Also, the standard library provides a binary search algorithm, lower_bound.

Is that code snippet complete? You should be getting an error that not all paths return a value. You end the function on an if-statement. I.e., when execution drops through the IF, there's no return value.

Please don't PM me with questions. Post them in the forums for everyone's benefit, and I can embarrass myself publicly.

You don't forget how to play when you grow old; you grow old when you forget how to play.

There is an implementation of binary search in the library: upper_bound


// Non portable bitwise ops.

Moreover, if you were to test for even or odd numbers (though, as pointed out, it isn't necessary), wouldn't it be easier (and more portable) to AND with 1 like so: (x & 1)? I think that communicates clearer intent.

Fastcall said everything that I was going to say. Not sure why his post was -1'ed.

x&1 or x%2 are the idiomatic ways of checking if an int is odd/even. The shift by 31 trick is much more confusing (and unecessary here).

As mentioned by Buckeye, you need a return -1; etc to handle the failure case.

There are several unusual things you are doing there.

I discussed binary search and how to do it better, in this article that I wrote about various searching techniques:

http://homepages.ihug.co.nz/~aurora76/Malc/Searching.htm

"In order to understand recursion, you must first understand recursion."
My website dedicated to sorting algorithms

Corrected according to feedback:


 
unsigned int BinarySearch(int data[], unsigned int elements, int key)
{
    if(elements==0)
    {
        cerr << "Binary search not able to perform on 0 element array";
        exit(-1);
    }
    unsigned int half_offset = elements/2 /*upper half smaller when even elements*/;
    unsigned int current_index = elements/2; 
    
        
    while(half_offset!=0)
    {
        if(data[current_index]==key)
            return current_index;
        else if(data[current_index]<key)
        {
            current_index += (half_offset+1)/2;
        }
        else if(data[current_index]>key)
        {
            current_index -= (half_offset+1)/2;
        }
        half_offset /= 2; // Truncated to floor
    }
    if(data[current_index]==key)
                return current_index;
    else
        cerr << "Finished array search without any matching results";
}
Intel Core 2 Quad CPU Q6600, 2.4 GHz. 3GB RAM. ATI Radeon HD 3400.

    if(data[current_index]==key)
                return current_index;
    else
        cerr << "Finished array search without any matching results";
    return <fail-value>; // perhaps -1 as suggested above
}

As mentioned before, you need to add a return <value> before the closing brace.

Please don't PM me with questions. Post them in the forums for everyone's benefit, and I can embarrass myself publicly.

You don't forget how to play when you grow old; you grow old when you forget how to play.

How do I choose a failure value in an unsigned int?

Intel Core 2 Quad CPU Q6600, 2.4 GHz. 3GB RAM. ATI Radeon HD 3400.

This topic is closed to new replies.

Advertisement