Binary Search Implementation in C++












2












$begingroup$


In computer science, binary search, also known as half-interval search or logarithmic search, is a search algorithm that finds the position of a target value within a sorted array.



The code returns true if an element is present in the array else returns false.



Any suggestion in improving the code is welcome.



Binary_Search.h



#ifndef BINARY_SEARCH_H
#define BINARY_SEARCH_H

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

#endif // BINARY_SEARCH_H


Binary_Search.cpp



#include <iostream>
#include "Binary_Search.h"

/* search a key in an array using binary search */
bool binary_search(int ar, int low, int high, int key)
{
int mid;

while ( low <= high )
{
// find the middle index
mid = low + ((high - low) >> 1);

if ( ar[mid] == key ) // key found
{
return true;
}
else if ( ar[mid] > key ) // key may be on the left half
{
high = mid - 1;
}
else if ( ar[mid] < key ) // key may be on the right half
{
low = mid + 1;
}
}

// key not found
return false;
}

int main()
{
int ar = {1, 7, 9, 10, 28, 28, 36, 49, 68, 99};

for ( int i = 0; i < sizeof(ar) / sizeof(ar[0]); ++i )
{
if ( binary_search(ar, 0, sizeof(ar) / sizeof(ar[0]) - 1, ar[i]) )
{
std::cout << ar[i] << " is present" << std::endl;
}
else
{
std::cout << ar[i] << " is not present" << std::endl;
}
}

int ar1 = {-1, 2, 3, 12, 23, 50, 90, 98, 100};
for ( int i = 0; i < sizeof(ar1) / sizeof(ar1[0]); ++i )
{
if ( binary_search(ar, 0, sizeof(ar) / sizeof(ar[0]) - 1, ar1[i]) )
{
std::cout << ar1[i] << " is present" << std::endl;
}
else
{
std::cout << ar1[i] << " is not present" << std::endl;
}
}
return 0;
}









share|improve this question











$endgroup$












  • $begingroup$
    Your formatting style looks weird and unusual. But that's actually not a point for code review, unless there are certain style guides set up.
    $endgroup$
    – πάντα ῥεῖ
    Jan 28 '17 at 11:44










  • $begingroup$
    Well if you are pointing to the braces then emacs gives that kind of formatting. I don't know whether that is good or not
    $endgroup$
    – Abhisek
    Jan 28 '17 at 11:53










  • $begingroup$
    Yes I'm talking about the brace indentation. IIRC you can choose/edit the templates emacs applies.
    $endgroup$
    – πάντα ῥεῖ
    Jan 28 '17 at 11:55










  • $begingroup$
    I'll search that, but why the default indentation behavior is strange.
    $endgroup$
    – Abhisek
    Jan 28 '17 at 11:59










  • $begingroup$
    The more usual style is that the opening brace is at the same indentation level as the conditional/loop condition statement, or starts with one blank offset after that statement, and the closing brace is always at the same indentation level as the conditional/loop condition statement.
    $endgroup$
    – πάντα ῥεῖ
    Jan 28 '17 at 12:02
















2












$begingroup$


In computer science, binary search, also known as half-interval search or logarithmic search, is a search algorithm that finds the position of a target value within a sorted array.



The code returns true if an element is present in the array else returns false.



Any suggestion in improving the code is welcome.



Binary_Search.h



#ifndef BINARY_SEARCH_H
#define BINARY_SEARCH_H

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

#endif // BINARY_SEARCH_H


Binary_Search.cpp



#include <iostream>
#include "Binary_Search.h"

/* search a key in an array using binary search */
bool binary_search(int ar, int low, int high, int key)
{
int mid;

while ( low <= high )
{
// find the middle index
mid = low + ((high - low) >> 1);

if ( ar[mid] == key ) // key found
{
return true;
}
else if ( ar[mid] > key ) // key may be on the left half
{
high = mid - 1;
}
else if ( ar[mid] < key ) // key may be on the right half
{
low = mid + 1;
}
}

// key not found
return false;
}

int main()
{
int ar = {1, 7, 9, 10, 28, 28, 36, 49, 68, 99};

for ( int i = 0; i < sizeof(ar) / sizeof(ar[0]); ++i )
{
if ( binary_search(ar, 0, sizeof(ar) / sizeof(ar[0]) - 1, ar[i]) )
{
std::cout << ar[i] << " is present" << std::endl;
}
else
{
std::cout << ar[i] << " is not present" << std::endl;
}
}

int ar1 = {-1, 2, 3, 12, 23, 50, 90, 98, 100};
for ( int i = 0; i < sizeof(ar1) / sizeof(ar1[0]); ++i )
{
if ( binary_search(ar, 0, sizeof(ar) / sizeof(ar[0]) - 1, ar1[i]) )
{
std::cout << ar1[i] << " is present" << std::endl;
}
else
{
std::cout << ar1[i] << " is not present" << std::endl;
}
}
return 0;
}









share|improve this question











$endgroup$












  • $begingroup$
    Your formatting style looks weird and unusual. But that's actually not a point for code review, unless there are certain style guides set up.
    $endgroup$
    – πάντα ῥεῖ
    Jan 28 '17 at 11:44










  • $begingroup$
    Well if you are pointing to the braces then emacs gives that kind of formatting. I don't know whether that is good or not
    $endgroup$
    – Abhisek
    Jan 28 '17 at 11:53










  • $begingroup$
    Yes I'm talking about the brace indentation. IIRC you can choose/edit the templates emacs applies.
    $endgroup$
    – πάντα ῥεῖ
    Jan 28 '17 at 11:55










  • $begingroup$
    I'll search that, but why the default indentation behavior is strange.
    $endgroup$
    – Abhisek
    Jan 28 '17 at 11:59










  • $begingroup$
    The more usual style is that the opening brace is at the same indentation level as the conditional/loop condition statement, or starts with one blank offset after that statement, and the closing brace is always at the same indentation level as the conditional/loop condition statement.
    $endgroup$
    – πάντα ῥεῖ
    Jan 28 '17 at 12:02














2












2








2





$begingroup$


In computer science, binary search, also known as half-interval search or logarithmic search, is a search algorithm that finds the position of a target value within a sorted array.



The code returns true if an element is present in the array else returns false.



Any suggestion in improving the code is welcome.



Binary_Search.h



#ifndef BINARY_SEARCH_H
#define BINARY_SEARCH_H

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

#endif // BINARY_SEARCH_H


Binary_Search.cpp



#include <iostream>
#include "Binary_Search.h"

/* search a key in an array using binary search */
bool binary_search(int ar, int low, int high, int key)
{
int mid;

while ( low <= high )
{
// find the middle index
mid = low + ((high - low) >> 1);

if ( ar[mid] == key ) // key found
{
return true;
}
else if ( ar[mid] > key ) // key may be on the left half
{
high = mid - 1;
}
else if ( ar[mid] < key ) // key may be on the right half
{
low = mid + 1;
}
}

// key not found
return false;
}

int main()
{
int ar = {1, 7, 9, 10, 28, 28, 36, 49, 68, 99};

for ( int i = 0; i < sizeof(ar) / sizeof(ar[0]); ++i )
{
if ( binary_search(ar, 0, sizeof(ar) / sizeof(ar[0]) - 1, ar[i]) )
{
std::cout << ar[i] << " is present" << std::endl;
}
else
{
std::cout << ar[i] << " is not present" << std::endl;
}
}

int ar1 = {-1, 2, 3, 12, 23, 50, 90, 98, 100};
for ( int i = 0; i < sizeof(ar1) / sizeof(ar1[0]); ++i )
{
if ( binary_search(ar, 0, sizeof(ar) / sizeof(ar[0]) - 1, ar1[i]) )
{
std::cout << ar1[i] << " is present" << std::endl;
}
else
{
std::cout << ar1[i] << " is not present" << std::endl;
}
}
return 0;
}









share|improve this question











$endgroup$




In computer science, binary search, also known as half-interval search or logarithmic search, is a search algorithm that finds the position of a target value within a sorted array.



The code returns true if an element is present in the array else returns false.



Any suggestion in improving the code is welcome.



Binary_Search.h



#ifndef BINARY_SEARCH_H
#define BINARY_SEARCH_H

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

#endif // BINARY_SEARCH_H


Binary_Search.cpp



#include <iostream>
#include "Binary_Search.h"

/* search a key in an array using binary search */
bool binary_search(int ar, int low, int high, int key)
{
int mid;

while ( low <= high )
{
// find the middle index
mid = low + ((high - low) >> 1);

if ( ar[mid] == key ) // key found
{
return true;
}
else if ( ar[mid] > key ) // key may be on the left half
{
high = mid - 1;
}
else if ( ar[mid] < key ) // key may be on the right half
{
low = mid + 1;
}
}

// key not found
return false;
}

int main()
{
int ar = {1, 7, 9, 10, 28, 28, 36, 49, 68, 99};

for ( int i = 0; i < sizeof(ar) / sizeof(ar[0]); ++i )
{
if ( binary_search(ar, 0, sizeof(ar) / sizeof(ar[0]) - 1, ar[i]) )
{
std::cout << ar[i] << " is present" << std::endl;
}
else
{
std::cout << ar[i] << " is not present" << std::endl;
}
}

int ar1 = {-1, 2, 3, 12, 23, 50, 90, 98, 100};
for ( int i = 0; i < sizeof(ar1) / sizeof(ar1[0]); ++i )
{
if ( binary_search(ar, 0, sizeof(ar) / sizeof(ar[0]) - 1, ar1[i]) )
{
std::cout << ar1[i] << " is present" << std::endl;
}
else
{
std::cout << ar1[i] << " is not present" << std::endl;
}
}
return 0;
}






c++ binary-search






share|improve this question















share|improve this question













share|improve this question




share|improve this question








edited Jan 29 '17 at 0:12









Jamal

30.3k11117227




30.3k11117227










asked Jan 28 '17 at 11:24









AbhisekAbhisek

227111




227111












  • $begingroup$
    Your formatting style looks weird and unusual. But that's actually not a point for code review, unless there are certain style guides set up.
    $endgroup$
    – πάντα ῥεῖ
    Jan 28 '17 at 11:44










  • $begingroup$
    Well if you are pointing to the braces then emacs gives that kind of formatting. I don't know whether that is good or not
    $endgroup$
    – Abhisek
    Jan 28 '17 at 11:53










  • $begingroup$
    Yes I'm talking about the brace indentation. IIRC you can choose/edit the templates emacs applies.
    $endgroup$
    – πάντα ῥεῖ
    Jan 28 '17 at 11:55










  • $begingroup$
    I'll search that, but why the default indentation behavior is strange.
    $endgroup$
    – Abhisek
    Jan 28 '17 at 11:59










  • $begingroup$
    The more usual style is that the opening brace is at the same indentation level as the conditional/loop condition statement, or starts with one blank offset after that statement, and the closing brace is always at the same indentation level as the conditional/loop condition statement.
    $endgroup$
    – πάντα ῥεῖ
    Jan 28 '17 at 12:02


















  • $begingroup$
    Your formatting style looks weird and unusual. But that's actually not a point for code review, unless there are certain style guides set up.
    $endgroup$
    – πάντα ῥεῖ
    Jan 28 '17 at 11:44










  • $begingroup$
    Well if you are pointing to the braces then emacs gives that kind of formatting. I don't know whether that is good or not
    $endgroup$
    – Abhisek
    Jan 28 '17 at 11:53










  • $begingroup$
    Yes I'm talking about the brace indentation. IIRC you can choose/edit the templates emacs applies.
    $endgroup$
    – πάντα ῥεῖ
    Jan 28 '17 at 11:55










  • $begingroup$
    I'll search that, but why the default indentation behavior is strange.
    $endgroup$
    – Abhisek
    Jan 28 '17 at 11:59










  • $begingroup$
    The more usual style is that the opening brace is at the same indentation level as the conditional/loop condition statement, or starts with one blank offset after that statement, and the closing brace is always at the same indentation level as the conditional/loop condition statement.
    $endgroup$
    – πάντα ῥεῖ
    Jan 28 '17 at 12:02
















$begingroup$
Your formatting style looks weird and unusual. But that's actually not a point for code review, unless there are certain style guides set up.
$endgroup$
– πάντα ῥεῖ
Jan 28 '17 at 11:44




$begingroup$
Your formatting style looks weird and unusual. But that's actually not a point for code review, unless there are certain style guides set up.
$endgroup$
– πάντα ῥεῖ
Jan 28 '17 at 11:44












$begingroup$
Well if you are pointing to the braces then emacs gives that kind of formatting. I don't know whether that is good or not
$endgroup$
– Abhisek
Jan 28 '17 at 11:53




$begingroup$
Well if you are pointing to the braces then emacs gives that kind of formatting. I don't know whether that is good or not
$endgroup$
– Abhisek
Jan 28 '17 at 11:53












$begingroup$
Yes I'm talking about the brace indentation. IIRC you can choose/edit the templates emacs applies.
$endgroup$
– πάντα ῥεῖ
Jan 28 '17 at 11:55




$begingroup$
Yes I'm talking about the brace indentation. IIRC you can choose/edit the templates emacs applies.
$endgroup$
– πάντα ῥεῖ
Jan 28 '17 at 11:55












$begingroup$
I'll search that, but why the default indentation behavior is strange.
$endgroup$
– Abhisek
Jan 28 '17 at 11:59




$begingroup$
I'll search that, but why the default indentation behavior is strange.
$endgroup$
– Abhisek
Jan 28 '17 at 11:59












$begingroup$
The more usual style is that the opening brace is at the same indentation level as the conditional/loop condition statement, or starts with one blank offset after that statement, and the closing brace is always at the same indentation level as the conditional/loop condition statement.
$endgroup$
– πάντα ῥεῖ
Jan 28 '17 at 12:02




$begingroup$
The more usual style is that the opening brace is at the same indentation level as the conditional/loop condition statement, or starts with one blank offset after that statement, and the closing brace is always at the same indentation level as the conditional/loop condition statement.
$endgroup$
– πάντα ῥεῖ
Jan 28 '17 at 12:02










3 Answers
3






active

oldest

votes


















3












$begingroup$

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;
}





share|improve this answer











$endgroup$













  • $begingroup$
    "Don't use >> 1 to represent a divide by 2." More importantly, don't use a right-shift to represent a divide with a signed integer, as the behavior is undefined. To make this work correctly, you need to cast the value to an unsigned integer. Then you could legally use a shift, but as you said, you should just do a division and let the compiler optimize it. (It will.)
    $endgroup$
    – Cody Gray
    Jan 29 '17 at 11:55










  • $begingroup$
    While I agree that using an iterator interface like STL does is nicer, I wouldn't advocate using template<typename I> for a function that clearly only should accept only one specific type of iterator, namely int * in this case.
    $endgroup$
    – G. Sliepen
    Jan 29 '17 at 12:00










  • $begingroup$
    @G.Sliepen: What about vector<int> or float or vector<float> or std::complex<int> or std::vector<std::complex<int>> The interface is valid for all these types.
    $endgroup$
    – Martin York
    Jan 29 '17 at 16:59










  • $begingroup$
    @LokiAstari: sure, but maybe that is not desired? In any case, the problem was not about making a template function, and the solution does not require a template function, so by suggesting it you risk adding confusion. You also did not provide an example of how to use your template function interface.
    $endgroup$
    – G. Sliepen
    Jan 29 '17 at 19:51



















2












$begingroup$

Avoid using std::endl



Use "n" instead. It is guaranteed to produce the correct newline character(s) on all platforms.
The problem with std::endl is that it not only adds a newline, it also flushes the output stream. If you have a lot of data to write to the output, this can cause a significant slowdown.



So instead of:



std::cout << ar[i] << " is present" << std::endl;


Write the following:



std::cout << ar[i] << " is presentn";





share|improve this answer









$endgroup$





















    0












    $begingroup$

    If you use high past end of the array, and replace



    while ( low <= high )


    with



    while ( low != high )


    you'll be able to search descending array by initial low > high values.






    share|improve this answer








    New contributor




    user2052436 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
    Check out our Code of Conduct.






    $endgroup$













      Your Answer





      StackExchange.ifUsing("editor", function () {
      return StackExchange.using("mathjaxEditing", function () {
      StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
      StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
      });
      });
      }, "mathjax-editing");

      StackExchange.ifUsing("editor", function () {
      StackExchange.using("externalEditor", function () {
      StackExchange.using("snippets", function () {
      StackExchange.snippets.init();
      });
      });
      }, "code-snippets");

      StackExchange.ready(function() {
      var channelOptions = {
      tags: "".split(" "),
      id: "196"
      };
      initTagRenderer("".split(" "), "".split(" "), channelOptions);

      StackExchange.using("externalEditor", function() {
      // Have to fire editor after snippets, if snippets enabled
      if (StackExchange.settings.snippets.snippetsEnabled) {
      StackExchange.using("snippets", function() {
      createEditor();
      });
      }
      else {
      createEditor();
      }
      });

      function createEditor() {
      StackExchange.prepareEditor({
      heartbeatType: 'answer',
      autoActivateHeartbeat: false,
      convertImagesToLinks: false,
      noModals: true,
      showLowRepImageUploadWarning: true,
      reputationToPostImages: null,
      bindNavPrevention: true,
      postfix: "",
      imageUploader: {
      brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
      contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
      allowUrls: true
      },
      onDemand: true,
      discardSelector: ".discard-answer"
      ,immediatelyShowMarkdownHelp:true
      });


      }
      });














      draft saved

      draft discarded


















      StackExchange.ready(
      function () {
      StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f153823%2fbinary-search-implementation-in-c%23new-answer', 'question_page');
      }
      );

      Post as a guest















      Required, but never shown

























      3 Answers
      3






      active

      oldest

      votes








      3 Answers
      3






      active

      oldest

      votes









      active

      oldest

      votes






      active

      oldest

      votes









      3












      $begingroup$

      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;
      }





      share|improve this answer











      $endgroup$













      • $begingroup$
        "Don't use >> 1 to represent a divide by 2." More importantly, don't use a right-shift to represent a divide with a signed integer, as the behavior is undefined. To make this work correctly, you need to cast the value to an unsigned integer. Then you could legally use a shift, but as you said, you should just do a division and let the compiler optimize it. (It will.)
        $endgroup$
        – Cody Gray
        Jan 29 '17 at 11:55










      • $begingroup$
        While I agree that using an iterator interface like STL does is nicer, I wouldn't advocate using template<typename I> for a function that clearly only should accept only one specific type of iterator, namely int * in this case.
        $endgroup$
        – G. Sliepen
        Jan 29 '17 at 12:00










      • $begingroup$
        @G.Sliepen: What about vector<int> or float or vector<float> or std::complex<int> or std::vector<std::complex<int>> The interface is valid for all these types.
        $endgroup$
        – Martin York
        Jan 29 '17 at 16:59










      • $begingroup$
        @LokiAstari: sure, but maybe that is not desired? In any case, the problem was not about making a template function, and the solution does not require a template function, so by suggesting it you risk adding confusion. You also did not provide an example of how to use your template function interface.
        $endgroup$
        – G. Sliepen
        Jan 29 '17 at 19:51
















      3












      $begingroup$

      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;
      }





      share|improve this answer











      $endgroup$













      • $begingroup$
        "Don't use >> 1 to represent a divide by 2." More importantly, don't use a right-shift to represent a divide with a signed integer, as the behavior is undefined. To make this work correctly, you need to cast the value to an unsigned integer. Then you could legally use a shift, but as you said, you should just do a division and let the compiler optimize it. (It will.)
        $endgroup$
        – Cody Gray
        Jan 29 '17 at 11:55










      • $begingroup$
        While I agree that using an iterator interface like STL does is nicer, I wouldn't advocate using template<typename I> for a function that clearly only should accept only one specific type of iterator, namely int * in this case.
        $endgroup$
        – G. Sliepen
        Jan 29 '17 at 12:00










      • $begingroup$
        @G.Sliepen: What about vector<int> or float or vector<float> or std::complex<int> or std::vector<std::complex<int>> The interface is valid for all these types.
        $endgroup$
        – Martin York
        Jan 29 '17 at 16:59










      • $begingroup$
        @LokiAstari: sure, but maybe that is not desired? In any case, the problem was not about making a template function, and the solution does not require a template function, so by suggesting it you risk adding confusion. You also did not provide an example of how to use your template function interface.
        $endgroup$
        – G. Sliepen
        Jan 29 '17 at 19:51














      3












      3








      3





      $begingroup$

      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;
      }





      share|improve this answer











      $endgroup$



      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;
      }






      share|improve this answer














      share|improve this answer



      share|improve this answer








      edited Jan 28 '17 at 23:50

























      answered Jan 28 '17 at 23:38









      Martin YorkMartin York

      73k486265




      73k486265












      • $begingroup$
        "Don't use >> 1 to represent a divide by 2." More importantly, don't use a right-shift to represent a divide with a signed integer, as the behavior is undefined. To make this work correctly, you need to cast the value to an unsigned integer. Then you could legally use a shift, but as you said, you should just do a division and let the compiler optimize it. (It will.)
        $endgroup$
        – Cody Gray
        Jan 29 '17 at 11:55










      • $begingroup$
        While I agree that using an iterator interface like STL does is nicer, I wouldn't advocate using template<typename I> for a function that clearly only should accept only one specific type of iterator, namely int * in this case.
        $endgroup$
        – G. Sliepen
        Jan 29 '17 at 12:00










      • $begingroup$
        @G.Sliepen: What about vector<int> or float or vector<float> or std::complex<int> or std::vector<std::complex<int>> The interface is valid for all these types.
        $endgroup$
        – Martin York
        Jan 29 '17 at 16:59










      • $begingroup$
        @LokiAstari: sure, but maybe that is not desired? In any case, the problem was not about making a template function, and the solution does not require a template function, so by suggesting it you risk adding confusion. You also did not provide an example of how to use your template function interface.
        $endgroup$
        – G. Sliepen
        Jan 29 '17 at 19:51


















      • $begingroup$
        "Don't use >> 1 to represent a divide by 2." More importantly, don't use a right-shift to represent a divide with a signed integer, as the behavior is undefined. To make this work correctly, you need to cast the value to an unsigned integer. Then you could legally use a shift, but as you said, you should just do a division and let the compiler optimize it. (It will.)
        $endgroup$
        – Cody Gray
        Jan 29 '17 at 11:55










      • $begingroup$
        While I agree that using an iterator interface like STL does is nicer, I wouldn't advocate using template<typename I> for a function that clearly only should accept only one specific type of iterator, namely int * in this case.
        $endgroup$
        – G. Sliepen
        Jan 29 '17 at 12:00










      • $begingroup$
        @G.Sliepen: What about vector<int> or float or vector<float> or std::complex<int> or std::vector<std::complex<int>> The interface is valid for all these types.
        $endgroup$
        – Martin York
        Jan 29 '17 at 16:59










      • $begingroup$
        @LokiAstari: sure, but maybe that is not desired? In any case, the problem was not about making a template function, and the solution does not require a template function, so by suggesting it you risk adding confusion. You also did not provide an example of how to use your template function interface.
        $endgroup$
        – G. Sliepen
        Jan 29 '17 at 19:51
















      $begingroup$
      "Don't use >> 1 to represent a divide by 2." More importantly, don't use a right-shift to represent a divide with a signed integer, as the behavior is undefined. To make this work correctly, you need to cast the value to an unsigned integer. Then you could legally use a shift, but as you said, you should just do a division and let the compiler optimize it. (It will.)
      $endgroup$
      – Cody Gray
      Jan 29 '17 at 11:55




      $begingroup$
      "Don't use >> 1 to represent a divide by 2." More importantly, don't use a right-shift to represent a divide with a signed integer, as the behavior is undefined. To make this work correctly, you need to cast the value to an unsigned integer. Then you could legally use a shift, but as you said, you should just do a division and let the compiler optimize it. (It will.)
      $endgroup$
      – Cody Gray
      Jan 29 '17 at 11:55












      $begingroup$
      While I agree that using an iterator interface like STL does is nicer, I wouldn't advocate using template<typename I> for a function that clearly only should accept only one specific type of iterator, namely int * in this case.
      $endgroup$
      – G. Sliepen
      Jan 29 '17 at 12:00




      $begingroup$
      While I agree that using an iterator interface like STL does is nicer, I wouldn't advocate using template<typename I> for a function that clearly only should accept only one specific type of iterator, namely int * in this case.
      $endgroup$
      – G. Sliepen
      Jan 29 '17 at 12:00












      $begingroup$
      @G.Sliepen: What about vector<int> or float or vector<float> or std::complex<int> or std::vector<std::complex<int>> The interface is valid for all these types.
      $endgroup$
      – Martin York
      Jan 29 '17 at 16:59




      $begingroup$
      @G.Sliepen: What about vector<int> or float or vector<float> or std::complex<int> or std::vector<std::complex<int>> The interface is valid for all these types.
      $endgroup$
      – Martin York
      Jan 29 '17 at 16:59












      $begingroup$
      @LokiAstari: sure, but maybe that is not desired? In any case, the problem was not about making a template function, and the solution does not require a template function, so by suggesting it you risk adding confusion. You also did not provide an example of how to use your template function interface.
      $endgroup$
      – G. Sliepen
      Jan 29 '17 at 19:51




      $begingroup$
      @LokiAstari: sure, but maybe that is not desired? In any case, the problem was not about making a template function, and the solution does not require a template function, so by suggesting it you risk adding confusion. You also did not provide an example of how to use your template function interface.
      $endgroup$
      – G. Sliepen
      Jan 29 '17 at 19:51













      2












      $begingroup$

      Avoid using std::endl



      Use "n" instead. It is guaranteed to produce the correct newline character(s) on all platforms.
      The problem with std::endl is that it not only adds a newline, it also flushes the output stream. If you have a lot of data to write to the output, this can cause a significant slowdown.



      So instead of:



      std::cout << ar[i] << " is present" << std::endl;


      Write the following:



      std::cout << ar[i] << " is presentn";





      share|improve this answer









      $endgroup$


















        2












        $begingroup$

        Avoid using std::endl



        Use "n" instead. It is guaranteed to produce the correct newline character(s) on all platforms.
        The problem with std::endl is that it not only adds a newline, it also flushes the output stream. If you have a lot of data to write to the output, this can cause a significant slowdown.



        So instead of:



        std::cout << ar[i] << " is present" << std::endl;


        Write the following:



        std::cout << ar[i] << " is presentn";





        share|improve this answer









        $endgroup$
















          2












          2








          2





          $begingroup$

          Avoid using std::endl



          Use "n" instead. It is guaranteed to produce the correct newline character(s) on all platforms.
          The problem with std::endl is that it not only adds a newline, it also flushes the output stream. If you have a lot of data to write to the output, this can cause a significant slowdown.



          So instead of:



          std::cout << ar[i] << " is present" << std::endl;


          Write the following:



          std::cout << ar[i] << " is presentn";





          share|improve this answer









          $endgroup$



          Avoid using std::endl



          Use "n" instead. It is guaranteed to produce the correct newline character(s) on all platforms.
          The problem with std::endl is that it not only adds a newline, it also flushes the output stream. If you have a lot of data to write to the output, this can cause a significant slowdown.



          So instead of:



          std::cout << ar[i] << " is present" << std::endl;


          Write the following:



          std::cout << ar[i] << " is presentn";






          share|improve this answer












          share|improve this answer



          share|improve this answer










          answered Jan 29 '17 at 12:04









          G. SliepenG. Sliepen

          1,272410




          1,272410























              0












              $begingroup$

              If you use high past end of the array, and replace



              while ( low <= high )


              with



              while ( low != high )


              you'll be able to search descending array by initial low > high values.






              share|improve this answer








              New contributor




              user2052436 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
              Check out our Code of Conduct.






              $endgroup$


















                0












                $begingroup$

                If you use high past end of the array, and replace



                while ( low <= high )


                with



                while ( low != high )


                you'll be able to search descending array by initial low > high values.






                share|improve this answer








                New contributor




                user2052436 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
                Check out our Code of Conduct.






                $endgroup$
















                  0












                  0








                  0





                  $begingroup$

                  If you use high past end of the array, and replace



                  while ( low <= high )


                  with



                  while ( low != high )


                  you'll be able to search descending array by initial low > high values.






                  share|improve this answer








                  New contributor




                  user2052436 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
                  Check out our Code of Conduct.






                  $endgroup$



                  If you use high past end of the array, and replace



                  while ( low <= high )


                  with



                  while ( low != high )


                  you'll be able to search descending array by initial low > high values.







                  share|improve this answer








                  New contributor




                  user2052436 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
                  Check out our Code of Conduct.









                  share|improve this answer



                  share|improve this answer






                  New contributor




                  user2052436 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
                  Check out our Code of Conduct.









                  answered 1 hour ago









                  user2052436user2052436

                  101




                  101




                  New contributor




                  user2052436 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
                  Check out our Code of Conduct.





                  New contributor





                  user2052436 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
                  Check out our Code of Conduct.






                  user2052436 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
                  Check out our Code of Conduct.






























                      draft saved

                      draft discarded




















































                      Thanks for contributing an answer to Code Review Stack Exchange!


                      • Please be sure to answer the question. Provide details and share your research!

                      But avoid



                      • Asking for help, clarification, or responding to other answers.

                      • Making statements based on opinion; back them up with references or personal experience.


                      Use MathJax to format equations. MathJax reference.


                      To learn more, see our tips on writing great answers.




                      draft saved


                      draft discarded














                      StackExchange.ready(
                      function () {
                      StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f153823%2fbinary-search-implementation-in-c%23new-answer', 'question_page');
                      }
                      );

                      Post as a guest















                      Required, but never shown





















































                      Required, but never shown














                      Required, but never shown












                      Required, but never shown







                      Required, but never shown

































                      Required, but never shown














                      Required, but never shown












                      Required, but never shown







                      Required, but never shown







                      Popular posts from this blog

                      Terni

                      A new problem with tex4ht and tikz

                      Sun Ra