Smallest Common Multiple unsolved

JavaScript
1

Tell us what’s happening:

My code doesn’t work for smallestCommons([23, 18]) for some strange reason; it’s not about the algorithm taking too much time (infine loop security) but the result is incorrect too. The result it gives is 2018940 (expected result is 6056820).
I found that: 6056820/2018940 = 3 (I don’t know if it helps)

The code gives good result for:

smallestCommons([1, 5])smallestCommons([5, 1])
smallestCommons([2, 10])
smallestCommons([1, 13])

My code JS so far

function smallestCommons(arr) {


      if (arr[1] < arr[0])    arr = [arr[1],arr[0]];

      let num = arr[1];
      let k = 1;
  
 for (let i= arr[0]; i<=arr[1]; i++) {

      if (num%i !=0) {
            k++;
            num = arr[1]*k; /* num must be a multple of arr[1] (biggest number --> 
                                          increases speed*/
            i=arr[0];
      }

 }

return num;

}

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

2

You are changing the value you are looping over, mid looping.
That’s usually a bad idea since it will give unpredictable result.

That chunk of code will keep i to a value of 19 for so many interaction.
What exactly was the idea behind that loop?

3

Hi Marmiz! Thanks for your answer.

My idea for this loop what to reinitialize i. Because we want to check if num can be divided by the whole range going from arr[0] to arr[1].

Otherwise i wouldn’t start from arr[0] (beginning of the loop).

What’s a better way to reinitialize the loop ?

Thanks!

4

That’s something you pretty much never want to do, and I never had seen a case for it in my experience.

Well, so why you just don’t create an array with all the number in that range, and loop over that?

Remeber that the final expression is executed after the body of a foor loop.
So what happens in your loop is

 - i = 18
 - 23 % 18 != from 0 --> YES
 - set i back to 18
 - loop-over 
 - for loop execute final expression, this means i = 19

 - 23 % 19 !== 0 --> YES
 -  set i back to 18
 - loop-over 
 - for loop execute final expression, this means i = 19

I know that you are updating other values in the loop condition, but your loop gets stuck until eventually you hit a number that is greater than 23, stopping the loop.
That doesn’t guaranteed that are all the number in the range.

Hope this make some clarity :slight_smile: