Intermediate Algorithm Scripting - Smallest Common Multiple

JavaScript
1

Tell us what’s happening:
Describe your issue in detail here.

If someone could shed some light on this, I would highly appreciate it.
The algorithm passes, but I get the warning about “potential infinite loop detected” for the larger test cases.
Is it very inefficient to do it this way?
Please post any suggestions on how I would test for this?

  **Your code so far**

function smallestCommons(arr) {
let top = Math.max(...arr)
let bottom = Math.min(...arr)
let newArr = [];

for (let i = bottom; i <= top; i++) {
  newArr.push(i)
}

let i = top;
while (true) {
  if (newArr.every(item => i % item === 0)) { 
    console.log(i)
    return i;
  } else {
    i++;
  }
}



}

smallestCommons([5,1]);

  **Your browser information:**

User Agent is: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/103.0.0.0 Safari/537.36

Challenge: Intermediate Algorithm Scripting - Smallest Common Multiple

Link to the challenge:

2

I had similar situations for some of my solutions for this section, and also for some Euler problems I solved.
I don’t know, sometimes it’s a good sign, that your loops logic should be refactored, sometimes it’s just fine.
I think if your solution is inefficient and too slow, test will fail.
If it’s passing, it’s OK.

btw, I’ve been told that this:

is bad practice. I bet you have warnings about loops because of it
Relevant thread

3

Yep… The GCD and lcm algorithm is the fasted approach I’ve seen.

Very much so.

4

Thank you. I dislike using while loops.
Note, this goes so much quicker with this adjustment:
In the else clause replace { i++ } with { i += top }

5

I generally don’t like to use while loops either, but that is not a good reason to misuse a for loop. That’s the typical alternative you’ll see people try to seek in this problem, so that good job avoiding that.

This while loop can be fixed up by putting a real exit condition instead of explicitly making an infinite loop. In this case, we can put a upper bound on the LCM.

6

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.