Confused with Smallest Common Multiple exercise

1

My plan of attack for this exercise was to sort the given range from large to small. Then divide the largest number by each other number in the range. As soon as it’s determined that a number is not a multiple the first number increases to it’s next multiple and the loop continues until a match is hit for all numbers. I’m having trouble translating this to code though!

function smallestCommons(arr) {

  var myArg = arguments[0].sort(function(a,b) { //SORT ARGS LARGE TO SMALL
    return b-a;
  });
  var myRange = [];
  var testNum = myArg[0];
  var commonFound = false;
  
  for (var i=myArg[0]; i>=myArg[1]; i--) { //GENERATE RANGE
    myRange.push(i);
  }

 while (commonFound === false) {
   for (var j=1; j<myRange.length-1; j++) {
     if (testNum % myRange[j] === 0) {
       if (j===myRange.length-1) {
       commonFound = true;
       }
       else {
         break;
       }
     }
     else {
       testNum += testNum;
     }
 }
 } 

  return testNum;
}

smallestCommons([4,1]);

My current code is giving me an infinite loop, but I don’t understand why. I’ve gone as far as to write out every step that it’s doing on paper and I don’t see where the snag is…!

2

My guess as to why you have an infinite loop is, when the for-loop hits break, the while loop runs again (there’s no other code after the for-loop to execute). Then the for-loop runs again, following the same thing it did before it hit break the last time.

3

What my intention was is that the break would trigger assuming the last testNum was a multiple, but the for loop hadn’t reached the end of the myRange array. I assumed that break would start the for loop again on the next iteration… rinse and repeat until it either finds something not divisible and increases the multiple or gets to the end of the array and everything has been divisible, thus ending the while loop. This doesn’t seem to be happening though; how could I change it so the for loop continues until the end of the array?

4

I think I’ve pinned it down. You get an infinite loop because your code never hits if (j===myRange.length-1). The reason is that your for-loop is set to run only while j<myRange.length-1. When j increments, then equals myRange.length-1, the for loop stops, and there’s no chance for that if condition to get checked. I changed the condition in the for-loop to j<myRange.length, and it ran smoothly, but the tests were still red.

1 Like
5

That was a really good catch, you were completely right there. It seems that was just one of the issues though, as I still got an infinite loop after correcting it. Another issue I noticed was that my testNum was growing exponentially, not by multiples of 4. I reorganized my code and came up with this solution:

//noprotect
function smallestCommons(arr) {

  var myArg = arguments[0].sort(function(a,b) { //SORT ARGS LARGE TO SMALL
    return b-a;
  });
  var myRange = [];
  var testNum = myArg[0];
  var commonFound = false;

  for (var i=myArg[0]; i>=myArg[1]; i--) { //GENERATE RANGE
    myRange.push(i);
  }

 while (!commonFound) {
   
   for (var j=1; j<myRange.length; j++) {
     if (testNum % myRange[j] !== 0) {
       commonFound = false;
       testNum += myArg[0];
       break;
     }
     else {
       commonFound = true; 
     }
 }
 } 
  return testNum;
}
smallestCommons([1,5]);

Now this passes, but it loads a bit slow, and i had to add the //noprotect comment to disable their infinite loop warning. Is this because I’ve take a more ‘brute force’ approach to the exercise, and thus it’s just running inefficiently?

6

Most likely. Interestingly though, It’s instantaneous when ran in repl.it, but you have to use // noprotect in the challenge.

7

Well that’s pretty strange, but it does make me feel better that it worked fast on the other site. Thanks for your assistance in getting me through this one!