# Smallest Common Multiple acting weird? Or just browser?

Tell us what’s happening:

Well this code should work or at least works on localhost, but it won’t in FCC. What gives?

This screenshot on my computer when code is executed locally.

``````function smallestCommons(arr) {

var x=1;

arr=arr.sort();

var arr1=[];

for(var i=arr[0];i<=arr[1];i++){

arr1.push(i);

}

var temp=0;

while(x){

x++;

for(var j=0;j<arr1.length;j++){

if(x%arr1[j]==0){

temp++;

}
else{
temp=0;
}

}

if(temp==arr1.length){

console.log(x);
break;
}

}

return x;
}

smallestCommons([1,5]);

``````

Your Browser User Agent is: `Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.139 Safari/537.36`.

https://www.freecodecamp.org/challenges/smallest-common-multiple

It is because the FCC tests use an infinite loop protection feature. While your code does not have an infinite loop, it takes long enough that it “thinks” there is an infinite loop and stops execution of the code. What this means is you have not created as efficient of a solution needed to prevent the infinite loop protection feature from stopping the code.

You have two choices:

1. Think about a more efficient algorithm to solve the challenge

2. Add //noprotect to the very first line of top of your code to bypass the infinite loop protection feature.

1 Like

Damn, i forgot about FCC take on loops … I recently read about algorithms that act like mine like described here: https://en.wikipedia.org/wiki/Big_O_notation . I also read about cutting in chunks, then “operate” on each chunk, then “unify” data collected so to speak. Don’t how to apply this on my algorithm yet.
I’ll try to “chunk it”, before use //noprotect.
Thanks , i’ll try in more for the next couple of days to decrease time needed for execution.

Tell us what’s happening:

Function works fine on local computer and executes in 330ms. While previous version of it got executed for ~1.4min on local machine. You can already can see improvement. But FCC says execution took 1241ms?! Does FCC do some background processes or what? Anyhow, i have no further recourse but to use //noprotect. If anyone knows how i can speed up execution from 330ms/FCC’s 1241ms to less time i’ll listen. At this point i have no new ideas … Here’s a pen: and in codepen execution time is >130ms which is peak not average time of execution. Edit1: Because it thinks it’s endless loop, it exits loop earlier so result is far lower then it should be.

``````function smallestCommons(arr) {

arr.sort();
var arr1=[];

for(var i=arr[0];i<=arr[1];i++){

arr1.push(i);

}

var x=1;

function check(arr1) {

if (x%arr1==0){
return x%arr1==0;
}
}

while(x){

x++;

if(arr1.every(check)==true){

break;
}

}
return x;
}

smallestCommons([1,5]);
``````

Your Browser User Agent is: `Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.139 Safari/537.36`.

Hi,

Your code does indeed satisfy all the challenge test cases but it has a few issues.

The infinite loop warning is because on the last test your function check() gets called 6056820 times, each time testing every value of your arr1 as a common factor. It works, but it works by brute force. You are testing lots of numbers that could not possibly be a solution. You need to quit testing numbers that could never be a common multiple. Hint to get you started: for [1,5] any solution would have to be either 5 or some multiple of 5.

Your sort of the array passed as a parameter does not do what you think. In all the test cases of the challenge you just got lucky. None of the numbers they picked would trip up the test because of your faulty sort. Try this in a console to demonstrate `[100,20].sort()`

Well i passed with slight modification of my original code as in original post.
Here it is on Pen

Does it pass without needing to bypass the infinite loop protection? It still seems to be looping a lot for the calculation.

If you are really stuck you should start looking at the code camp challenge guide
https://forum.freecodecamp.org/t/freecodecamp-algorithm-challenge-guide-smallest-common-multiple/16075

Also, I don’t think you have really fixed the sort issue . I think you have just configured your loops to work with the largest number first after reversing the array. To use the .sort function on numbers you will need to create your own comparison function. Look on MDN for the example using numbers not strings.

Yes, it passes it without //noprotection.
Well, i actually measured speed of my and several other people’s code execution timing including" camperbot’s" Basic , Intermediate and Advanced Code Solution. My code execution speed is similar of Intermediate code speed, around ~20ms, sometimes more, sometimes less. Also declaring and even initializing variables outside of loops(where possible) also cuts down on execution time … Here’s smallestCommons You can test it and see it for yourself.

Although your code happens to pass the FCC tests (which are not as robust as they should be), your code does not give the correct response for the following:

``````  console.log(smallestCommons([2, 10])); //2520
``````

In fact, it returns NaN.

If in “arr.sort().reverse();” i erase reverse() it shows proper result i.e 2520, but then some other other calls get scrambled so to speak … or if i change starting point in “for (var i = arr[0]; i <= arr[1]; i++)” from arr[0] to arr[1] and ending point from arr[1] to arr[0] it behaves like i stated above. It perplexes me very much for why it does it behave like that?

Well, it’s kind a stupid … But somehow, .sort() didn’t do it’s job. It didn’t sort it in ascending manner. Whole code is leaning on proper sorting of initial array. Just needed to add an compare function as an parameter for .sort() method. Here’s a Pen

Yes, @alhazen1 mentioned this fact in his two previous responses. Glad you got it figured out.