Smallest Common Divisible

Folks,

I am trying FCC Intermediate Algorithm Scripting: Smallest Common Multiple and have come up with my own code. It works fine with all the test but [23, 18]. Any idea why the following code does not return the correct result for [23, 18]. https://jsconsole.com/ and https://js.do/ sites run the code fine and output the correct results. I have come across some similar issues on FCC challenges.

function smallestCommons(arr) {
 let scm = 0
 let arr2 = [Math.min(...arr)];
  for(let i = Math.min(...arr)+1; i <= Math.max(...arr); i++) {
   arr2.push(i);
}
for (let k = 1; k <= arr2.reduce((x,y) => x*y); k++) {
  if(arr2.every((n)=>{return (k % n === 0); })) {
   scm = k;
   break;
  }
}
  return scm
}

It seems your code is taking too much to run and triggering the infinite loop protection and so is being stopped midway.

You may need to refactor your code

Why https://js.do/ site runs it fine but fcc.org doesn’t

because it seems that the other editor you are using doesn’t have an infinite loop protection, or the code can run for more time before being stopped

1 Like

During the execution of a for loop the condition gets evaluated before each loop iteration.

This means that in general you want to use some kind of constant value to avoid general waste of resources.

This means that all your conditions

i <= Math.max(...arr);
arr2.reduce((x,y) => x*y)

Gets calculated at each interaction, when they could be easily calculated once before execution and then used :slight_smile:

Hope this helps :+1:

edit: with just that changes I almost halved the execution time, I still fear will be over 100ms

1 Like

Tried all these - not working again… I don’t quite get the infinite part… this loop has to run 6056820 times to return the result which is 6056820. the product of all number in the array [18, 19, 20, 21, 22, 23] is 72681840 which is 12 times bigger than 6056820. Well I understand the time part but it eventually should finish as it is NOT a loop that runs forever.

How can I check what is the run timeout ?

@CodeLearner i think the default protection is > 100ms.
Last time I tried your function I was a little bit above that value.

You can disable the protection if you want, I think all you need is adding //noprotect at the top of your code.
However do it at your own risk.

That said consider that perhaps it’s also a way to force you to refactor your solution, that yes, works, but at what cost?

As you said

this loop has to run 6056820 times to return the result which is 6056820

In theory that’s not true, at least all the number from 0 to N where N is your lowest boundary.
Maybe you can refactor it a bit more? :slight_smile:

//noprotect doesn’t work