Smallest Common Multiple (Help)

Smallest Common Multiple (Help)
0

#1

The code below passes first 4 tests but not the last 2 of them. Can anyone explain why?
smallestCommons([1, 5]) should return a number.

Passed
smallestCommons([1, 5]) should return 60.

Passed
smallestCommons([5, 1]) should return 60.

Passed
smallestCommons([2, 10]) should return 2520.

smallestCommons([1, 13]) should return 360360.
smallestCommons([23, 18]) should return 6056820.

Your code so far


function smallestCommons(arr) {
  arr.sort((a,b) => a - b);
  let allNumbers = [];
  for (let i = arr[0]; i <= arr[1]; i++) {
    allNumbers.push(i);
  }

  for (let j = 1; j < j + 1; j++) {
    if (allNumbers.every(x => j % x == 0 )) {
      return j;
    }
  }
}

smallestCommons([1,13]);

Your browser information:

User Agent is: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.100 Safari/537.36.

Link to the challenge:
https://learn.freecodecamp.org/javascript-algorithms-and-data-structures/intermediate-algorithm-scripting/smallest-common-multiple


#2

I ran into similar issues with my challenge the first time. I believe, though maybe someone else can confirm, that while your solution technically works, it is incredibly inefficient as it has to loop over every single number.

I was able to get around this by searching around and finding a far more efficient way to tackle the same problem.

One way to improve it is to think about the numbers that are not divisible by the higher number won’t be a match, so instead of looping through everything, you can start at the highest number given and then increment by that number instead of by 1.

Keep building on this and you’ll get more efficient and pass it :slight_smile:


#3

Yes now I can see it is very inefficient way to achieve the goal. Thank you and I will try to optimize it. But could you see that why it can’t pass all the test whereas it can pass some other?


#4

As I stated, I think it would theoretically eventually pass given enough iterations. But as it stands its virtually an infinite loop and therefore will never give the answer.

Take this example:

Using your current code, the number will start at 1, and then loop through every number greater than 1 until it finds a number that is the smallest common multiple of 1 through 13. Since that number is 360,360, your code is:

Looping through every number from 1 to 360,360 and for each number between them, looping from 1 to 13 and testing if the number is divisible. So it is looping a total of almost 4.7 million times. This is exponentially more in the following example.

I believe the console fCC uses is treating this as basically an infinite loop and either stopping it, or very slowly executing it.