0
\$\begingroup\$

I have implemented binary search algorithm in JavaScript:

var myArray = [], 
    searchNum = undefined;

// below function is used to capture the 
// commandline parameters for array and the
// number to be searched
(function(){
    process.argv.forEach(function (val, index, array) {
        var idx = 0, ar = undefined;

        try{

            // get the commandline argument for 
            // array values 
            if(index === 2){
                myArray = myArray.concat(val.split(",").map(function(num){
                    return parseInt(num);
                }));    
            }

            // third index is the number to be searched.
            if(index === 3){
                searchNum = parseInt(val)
            }

        }catch(e){
            console.log(e)
        }

    });
})();

console.log(" SEARCH NUMBER ",searchNum," in array ",myArray);

console.log(binary_search(myArray,searchNum,0,myArray.length));

function binary_search(numberArray, numberToSearch, lowIndex, maxIndex){
    var totalLength = maxIndex - lowIndex;

    var midIndex = parseInt(totalLength/2),
        str = "";

    /*
        If Lower Index is equal to Higher Index,
        that means number is not found and hence there is 
        a collision in pointers and hence return 
        as 'Can't be found'
    */  
    if(lowIndex === maxIndex){
        return "can't be found";
    }   

    /*
        setting the actual middle index 
        by adding the computed middle index with lower index.
    */
    midIndex = lowIndex + midIndex;

    // if number found
    if(numberArray[midIndex] === numberToSearch){

        str = " Number "+numberToSearch+" found at position "+midIndex;

        return str;

    // if number in middle index is less than the number to be searched
    // set the lower Index to new value i.e. a index position next higher to 
    // middle Index 
    }else if(numberArray[midIndex] < numberToSearch){

        lowIndex = midIndex + 1;


    // number to be searched is less than the number present at middle Index
    // set new maxIndex value i.e. index which is previous position to the
    // middle index     
    }else if(numberArray[midIndex] > numberToSearch){

        maxIndex = midIndex;

    }else{

        return "can't be found";

    }

    return binary_search(numberArray, numberToSearch, lowIndex, maxIndex);
} // end of method binary_search

When I run the above code the output is as follows,

E:\RahulShivsharan\MyPractise\DesignPatternsInJavaScript>node binarySearch.js 12,34,56,78,90 24
 SEARCH NUMBER  24  in array  [ 12, 34, 56, 78, 90 ]
can't be found

E:\RahulShivsharan\MyPractise\DesignPatternsInJavaScript>node binarySearch.js 12,34,56,78,90 34
 SEARCH NUMBER  34  in array  [ 12, 34, 56, 78, 90 ]
 Number 34 found at position 1

Can you please review my code and please suggest me if there is any room for further improvement.

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Parsing command line arguments

I mentioned this in a previous review, it seems I wasn't specific enough, but this is an inefficient and unnatural way to parse command line arguments:

    process.argv.forEach(function (val, index, array) {
        var idx = 0, ar = undefined;

        try{

            // get the commandline argument for 
            // array values 
            if(index === 2){
                myArray = myArray.concat(val.split(",").map(function(num){
                    return parseInt(num);
                }));    
            }

            // third index is the number to be searched.
            if(index === 3){
                searchNum = parseInt(val)
            }

        }catch(e){
            console.log(e)
        }

    });

What's wrong with it?

  • For each index 0, 1, 2, 3, the loop compares the index against 2 and 3 repeatedly
  • If there are not enough arguments, the loop should not even begin
  • The command line parsing logic fails its purpose by not failing in case the arguments are invalid

The natural way to parse would be:

  • Check process.argv.length and fail if invalid
  • Check the type of each argument and fail if invalid

Consider this alternative, no looping, nice and simple:

function parseArgs(prog, argv) {
  if (argv.length != 2) {
    throw `usage: node ${prog} NUMS_CSV NUM`;
  }

  function validInt(s) {
    var num = parseInt(s, 10);
    if (isNaN(num)) {
      throw "Not a valid number: " + s;
    }
    return num;
  }

  return {
    nums: argv[0].split(',').map(validInt),
    target: validInt(argv[1])
  };
}

var args = parseArgs(process.argv[1], process.argv.slice(2));

Parsing integers

When using parseInt, it's recommended to specify the radix parameter, because although base 10 is a common default, it's not guaranteed across different implementations. So to parse base 10 numbers, write parseInt(x, 10) instead of parseInt(x).

API design

The binary_search function searches for an element and returns two kinds of strings as result:

  • can't be found
  • Number N found at position X

This is very limiting. It would be better to have binary_search return an index or -1 if not found, and move the logic of formatting a string result into a dedicated function.

\$\endgroup\$
2
  • \$\begingroup\$ Why do you recommend specifying the radix 10 for parseInt when that is the default? \$\endgroup\$
    – kamoroso94
    Commented Jul 10, 2017 at 12:26
  • 1
    \$\begingroup\$ @kamoroso94 thanks for pointing out, I clarified. According this page, the default is not always 10, that's why it's good to write explicitly always. \$\endgroup\$
    – janos
    Commented Jul 10, 2017 at 12:33

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.