Return to Answer

##Interface

Passing an array and then two indexes is a very C style of interface. In C++ it is much more common to pass two iterators. An iterator is a generalized form of pointer.

So rather than:

bool binary_search(int ar[], int low, int high, int key);

I would use:

template<typename I>
bool binary_search(I low, I high, int key);

Also rather than pointing at the first and last elements iterator ranges use a first and one past the last. This makes calculating sizes easier.

###Code Review

Don't use >> 1 to represent a divide by 2. The point of high level code is to write it so that it is easy for humans to read. That is not obvious. Also the compiler can do these micro optimizations much better than you. So don't try and confuse it. Just write code in the most readable way possible.

Don't use sizeof(ar) / sizeof(ar[0]) it is so easy to break as array collapse into pointers at the drop of a hat. Use std::size() which works for arrays/containers but will fail to compile for pointers (which is what you want).

But if you switch to using Iterator based interface then you should use std::begin() and std::end().

###Style

Your bracing style is uncommon. But not so egregious that I would complain about. Normally brace style is defined by a local style guide. So if you are in a compnany or project just check that.

 // Most common C++ styles
 if ()
 {
     Statement;
 }

 // or
 if () {
     Statement;
 }

Interface

Passing an array and then two indexes is a very C style of interface. In C++ it is much more common to pass two iterators. An iterator is a generalized form of pointer.

So rather than:

bool binary_search(int ar[], int low, int high, int key);

I would use:

template<typename I>
bool binary_search(I low, I high, int key);

Also rather than pointing at the first and last elements iterator ranges use a first and one past the last. This makes calculating sizes easier.

Code Review

Don't use >> 1 to represent a divide by 2. The point of high level code is to write it so that it is easy for humans to read. That is not obvious. Also the compiler can do these micro optimizations much better than you. So don't try and confuse it. Just write code in the most readable way possible.

Don't use sizeof(ar) / sizeof(ar[0]) it is so easy to break as array collapse into pointers at the drop of a hat. Use std::size() which works for arrays/containers but will fail to compile for pointers (which is what you want).

But if you switch to using Iterator based interface then you should use std::begin() and std::end().

Style

Your bracing style is uncommon. But not so egregious that I would complain about. Normally brace style is defined by a local style guide. So if you are in a compnany or project just check that.

 // Most common C++ styles
 if ()
 {
     Statement;
 }

 // or
 if () {
     Statement;
 }

##Interface

Passing an array and then two indexes is a very C style of interface. In C++ it is much more common to pass two iterators. An iterator is a generalized form of pointer.

So rather than:

bool binary_search(int ar[], int low, int high, int key);

I would use:

template<typename I>
bool binary_search(I low, I high, int key);

Also rather than pointing at the first and last elements iterator ranges use a first and one past the last. This makes calculating sizes easier.

###Code Review

Don't use >> 1 to represent a divide by 2. The point of high level code is to write it so that it is easy for humans to read. That is not obvious. Also the compiler can do these micro optimizations much better than you. So don't try and confuse it. Just write code in the most readable way possible.

Don't use sizeof(ar) / sizeof(ar[0]) it is so easy to break as array collapse into pointers at the drop of a hat. Use std::size() which works for arrays/containers but will fail to compile for pointers (which is what you want).

But if you switch to using Iterator based interface then you should use std::begin() and std::end().

##Interface

Passing an array and then two indexes is a very C style of interface. In C++ it is much more common to pass two iterators. An iterator is a generalized form of pointer.

So rather than:

bool binary_search(int ar[], int low, int high, int key);

I would use:

template<typename I>
bool binary_search(I low, I high, int key);

Also rather than pointing at the first and last elements iterator ranges use a first and one past the last. This makes calculating sizes easier.

###Code Review

Don't use >> 1 to represent a divide by 2. The point of high level code is to write it so that it is easy for humans to read. That is not obvious. Also the compiler can do these micro optimizations much better than you. So don't try and confuse it. Just write code in the most readable way possible.

Don't use sizeof(ar) / sizeof(ar[0]) it is so easy to break as array collapse into pointers at the drop of a hat. Use std::size() which works for arrays/containers but will fail to compile for pointers (which is what you want).

But if you switch to using Iterator based interface then you should use std::begin() and std::end().

###Style

Your bracing style is uncommon. But not so egregious that I would complain about. Normally brace style is defined by a local style guide. So if you are in a compnany or project just check that.

 // Most common C++ styles
 if ()
 {
     Statement;
 }

 // or
 if () {
     Statement;
 }

##Interface

Passing an array and then two indexes is a very C style of interface. In C++ it is much more common to pass two iterators. An iterator is a generalized form of pointer.

So rather than:

bool binary_search(int ar[], int low, int high, int key);

I would use:

template<typename I>
bool binary_search(I low, I high, int key);

Also rather than pointing at the first and last elements iterator ranges use a first and one past the last. This makes calculating sizes easier.

###Code Review

Don't use >> 1 to represent a divide by 2. The point of high level code is to write it so that it is easy for humans to read. That is not obvious. Also the compiler can do these micro optimizations much better than you. So don't try and confuse it. Just write code in the most readable way possible.

##Interface

Passing an array and then two indexes is a very C style of interface. In C++ it is much more common to pass two iterators. An iterator is a generalized form of pointer.

So rather than:

bool binary_search(int ar[], int low, int high, int key);

I would use:

template<typename I>
bool binary_search(I low, I high, int key);

Also rather than pointing at the first and last elements iterator ranges use a first and one past the last. This makes calculating sizes easier.

###Code Review

Don't use >> 1 to represent a divide by 2. The point of high level code is to write it so that it is easy for humans to read. That is not obvious. Also the compiler can do these micro optimizations much better than you. So don't try and confuse it. Just write code in the most readable way possible.

Don't use sizeof(ar) / sizeof(ar[0]) it is so easy to break as array collapse into pointers at the drop of a hat. Use std::size() which works for arrays/containers but will fail to compile for pointers (which is what you want).

But if you switch to using Iterator based interface then you should use std::begin() and std::end().