3
\$\begingroup\$

Please review my code for a puzzle to find next bigger number with same combination of digits.

so if the number is 156432 the next bigger number is 162345

I wrote a program which is as follows,

// converts a number
// to an number array
// i.e. 13452 = [1,3,4,5,2]
var convertToNumArray =  function(num){

    var numList = [], 
        flag = true,
        n = 0;

    do{
         n = num % 10; // get the remainder value
         numList.push(n);
         num = parseInt(num/10); // get the quotient value in division

         if(num <= 0){
             flag = false;  
         }

     }while(flag === true);

     numList = numList.reverse();

     return numList;
}


// gets the next value
function getNext(num){
    var numArray = convertToNumArray(num), // gets the number array; example [1,3,4,5,2]
        totalLength = numArray.length - 1, 
        nextIndex = 0,
        leastIndex = 0,
        prevDiff = 0, 
        diff = 0, 
        swapIndex = 0, 
        temp = 0,
        str = "";

    // iterate from right most number to find out the 
    // number which is less than previous one.
    // for [1,3,4,5,2] the number is 4.     
    for(var index = totalLength; index >= 1; index--){
        nextIndex = (index - 1);

        if(numArray[nextIndex] < numArray[index]){
            leastIndex = nextIndex;
            break;
        }else{
            continue;
        }
    }

    // iterate from right to check the number which is closest
    // and greater then the number we found from above loop.
    // so in our case 5 is the closes to 4  
    for(index = totalLength; index > leastIndex; index--){
        diff = numArray[index] - numArray[leastIndex];

        if(index === totalLength){          
            prevDiff = (diff  < 0) ? 9999 : diff;
            closestNum = numArray[index];
            swapIndex = index;  
        }else{
            if(diff >= 0 && diff < prevDiff){               
                prevDiff = diff;
                swapIndex = index;
            }
        }

    }
    // as 5 is the closest to 4. Replace them
    // so the conversion will be from [1,3,4,5,2] to [1,3,5,4,2]
    temp = numArray[swapIndex];
    numArray[swapIndex] = numArray[leastIndex];
    numArray[leastIndex] = temp;


    /*
        Now sort the sub array i.e. the numbers falling right 
        to the replaced position.
        i.e. [1,3,5,4,2] so the sub array is [4,2].
        so sort in ascending order hence [2,4]
        so the complete number is 
        [1,3,5,2,4].
    */

    // bubble sort on sub array
    do{ 

        for(index = leastIndex + 1; index < totalLength; index++){
            nextIndex = index + 1;
            if(numArray[index] > numArray[nextIndex]){
                 temp = numArray[index];
                 numArray[index] = numArray[nextIndex];
                 numArray[nextIndex] = temp;  
            }
        }
        totalLength--; 
    }while(totalLength > (leastIndex + 1));

    // convert the array to string
    for(index = 0; index < numArray.length; index++){
        str += numArray[index];
    } 

    return parseInt(str);

}// end of getNext 


var nr = undefined;

// capturing from commandline
// example : node logic.js 12345
(function(){
    process.argv.forEach(function (val, index, array) {
        if(index === 2)
            nr = array[index];
    });
})();


console.log(nr+" => "+getNext(nr));

When I run the above program the output is

 E:\RahulShivsharan\MyPractise\DesignPatternsInJavaScript>node logic.js 156432
 156432 => 162345

 E:\RahulShivsharan\MyPractise\DesignPatternsInJavaScript>

Please review my code and suggest my any improvements in the logic or in approach or in the style of coding.

Thanks

\$\endgroup\$
1
\$\begingroup\$

You convert number to array can be simplified by first converting the number to a string and then splitting on nothing:

var convertToNumArray = function(num){
    return num.toString().split('')
}

This will of course convert it to an array of strings, but that will work perfectly fine for your purposes (or you can always throw in a map to convert back to ints).

Your first loop (noted with this comment):

// iterate from right most number to find out the 
// number which is less than previous one.

You don't need the else { continue }. It is at the end of the loop, so continuing is guaranteed. Sometimes explicit is nice but I would say not to the extend of writing code that doesn't actually do anything: that just makes for more code to read through.

Also, I don't think you have accounted for the possibility of a number that doesn't have a next value. For instance, how does your code handle 54321? Your swap index will stay at the initialized value of zero. Either way, I think the smart thing to do is to detect this event happening and exit prematurely, as you already have the answer: there is no room for changes.

Your second for loop looks solid. I might organize it a little differently but no actual complaints: just a different coding style.

For your last bit, I wouldn't do the sort yourself. Use the built in javascript sort. You can slice off the end of the array, use sort() (keep in mind that it sorts asciibetically, but that shouldn't hurt you here), and then splice it back onto the end.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.