Tell us what’s happening:
The code below can find the smallest common multiple up to [1, 15]. It will not function after it gets to more than [1, 16]
if I take away the if ( y % min == 0 &&; y % max == 0 ) condition on line 8 and tweak the for loop to include min and max number in the array. I will not function after [1,13]
It would like to know is this the problem of my code or the code execution reaches the limit of the output because there are too many loops going on? I don’t get it how my code is not correct after reaching a certain number parameter. The code should work in all numbers.3
Your code so far
function smallestCommons(arr) {
let max = Math.max(arr[0], arr[1]);
let min = Math.min(arr[0], arr[1]);
let i = 0;
let y = 1;
while (i < 1){
let k = 0;
if ( y % min == 0 && y % max == 0 ){
for (let x = min+1; x< max; x++){
if (y % x == 0 ){
k++;
}
}
}
if (k == (max - min -1)) {
i++;
return y;
}
else {
y++
}
}
}
console.log(smallestCommons([2, 14]));
Your browser information:
User Agent is: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.110 Safari/537.36.
So taking your code and setting it up on repl.it (I set up a pure javascript environment over there for testing purposes), if I run to [1,13] I get the following error:
RangeError: Potential infinite loop. You can disable this from settings.
at smallestCommons:6:79
at eval:25:1
at eval
at new Promise
Which basically means your algorithm is not particularly efficient. It may not be WRONG, but it over-taxes the resources allocated to this browser window.
I see, so my answer should be correct despite very inefficient thus costs potential infinite loop. I tried to change it to a more simple version of code with just one do while loop. This time it can get to [1,16] but still not a number larger than that.
I wonder does the challenge not want us to perform a loop that tries literally integers from 1 to the SCM? Is looping millions of integers in a loop not commonly acceptable in JavaScript? Thanks
function smallestCommons(arr) {
let max = Math.max(arr[0], arr[1]);
let min = Math.min(arr[0], arr[1]);
let arrr = [];
for (let j = min; j <= max; j++){
arrr.push(j);
}
let y = 1;
do {
if (arrr.every(x => y % x == 0) == true) {
return y
}
else {
y++
}
} while (true);
}
Actually, there are FAR more efficient means of doing this. Part of the challenge is researching ways to make this a less computationally intensive process. For example, the intermediate solution in the Hint section references the Euclidean Algorithm. While you could simply parrot their solution, you really don’t want to do that, as you wouldn’t really be learning – but you CAN research the algorithm itself and see if you can find a way to implement it. Their solution brings [1, 25] from more than four million iterations of multiple loops to about forty.
For my own edification, and to be clear I’m not condoning cheating!! I took the intermediate solution, ran it on repl.it, and added console.log statements here and there to actually see what was happening. Might be worth the while.
I’ve been searching for and thinking of ways to possibly lowering the code execution time. I finally can be able to pass the challenge by changing the y variable in line 10 from y = 1 to y = min*max. And change the y++ to y+= max. And actually, It passes the test with just changing y++ to y+=max. I guess that decreases the number of loops exponentially.
While I am able to pass this test. What I more curious is the limitation of JavaScript. Does it mean that I’ve to beware of the fact that the number of loops in for or while loop execution can’t be over a million? does it mean that the same code might not execute equally in different browsers or computer because they each have different limit to consider the code as a potential infinite loop?
Finally thanks again snowmonkey for saving my day again!