Smallest Common Multiple Infinite Loop Problem

1

Here’s my code:

/* function to find the sequential numbers (from
greatest to lowest) between the two given values. 
With the passed values included*/
function findSeq(max, min){
  var seqArr = [];
  for(var i = max; i >= min; i--){
    seqArr.push(i);
  }
 return seqArr;
}

function findSCM(n1, n2){
  var bool = true;
  var i = 1;
  var m1 = [], m2 = [];
  var sCM;
  
  //while SCM for n1 and n2 not found
  while(bool){
    m1.push(n1 * i);
    m2.push(n2 * i);
    
    //check if SCM found
    if(m1.indexOf(m2[i]) !== -1){
      sCM = m2[i];
      break;
    }else if(m2.indexOf(m1[i]) !== -1){
      sCM = m1[i];
      break;
    }
    
    i++;
  }
  
  return sCM;
}

function smallestCommons(arr) {
  //find the max, min value from arr
  var max, min;
  
  if(arr[0] > arr[1]) {
    max = arr[0];
    min = arr[1];
  }
  else if(arr[0] < arr[1]){
    max = arr[1];
    min = arr[0];
  }
  else return arr[0]; //if they're equal return one of them
  
  arr = findSeq(max, min);
  
  return arr.reduce(function(a, b){
    return findSCM(a,b);
  });
}

I’m getting this error: Error: Potential infinite loop at line 20.

I can’t think of another way to solve it, without getting into this infinite loop problem.

Any hints or suggestions?

2

1. Infinite loop problem : Argument to indexOf is ‘undefined’; thus, it won’t match with any number. As a result, it is not possible to break out from the loop.
Let,
n1 = 1
n2 = 3

By definition of SCM, we expect the correct answer to be
3

Suppose, we are on 3rd iteration just before i is incremented.
m1 = [1, 2, 3]
m2 = [3, 6, 9]
i = 3

Since i is 3, you are accessing 4th element of both arrays which is ‘undefined’.

2. You are solving the wrong problem
The argument passed to findSCM function represents inclusive range. If [1, 3] is given, it is asking for SCM among 1, 2, and 3.

3

1. Infinite loop problem : About this, I’m thinking of adding a second counter that starts from zero.

2. You are solving the wrong problem: Doesn’t reduce() do this (iterating over the array, and finding the SCM of the SCM of the last two elements with the next element.

Edit: I added a second counter for the array elements that starts from zero and I passed the 1st 3 tests, but not the final 2 tests. I think it’s because the code is taking too long time or has too many steps.

4
  1. I don’t really get it
  2. If you have range function which will expand [1, 3] to [1, 2, 3].

If your code gets shut down by grader, then your algorithm is too slow.

5
  1. Here’s the updated code for the findSCM() method:
unction findSCM(n1, n2){
  var bool = true;
  var i = 1, j = 0;
  var m1 = [], m2 = [];
  var sCM;
  
  //while SCM for n1 and n2 not found
  while(bool){
    m1.push(n1 * i);
    m2.push(n2 * i);
    
    //check if SCM found
    if(m1.indexOf(m2[j]) !== -1){
      sCM = m2[j];
      break;
    }else if(m2.indexOf(m1[j]) !== -1){
      sCM = m1[j];
      break;
    }
    
    i++;
    j++;
  }
  
  return sCM;
}

2 you mean like this function (which is already in my code):

function findSeq(max, min){
  var seqArr = [];
  for(var i = max; i >= min; i--){
    seqArr.push(i);
  }
 return seqArr;
}

If my algorithm is slow, how do I improve it?

6

3.If your algorithm is too slow, you need a new plan; but before doing that, you need to find out about why your algorithm is slow.