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.
I already asked this question, but i’m ask it again with additional clarification:
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.
Your code so far
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 information:
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.
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()
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.
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
