Smallest Common Multiple - What is so inefficient about my code?

Tell us what’s happening:

The first solution I wrote (that I won’t share) gave good results until the last test. I looked up the topic on this forum and read about how it can stop your loop if the code isn’t efficient (it thinks it’s an infinite loop).

I couldn’t figure out how to do it more efficiently, so I looked ahead at the spoiler. The code I’m submitting now is based on that. In fact, as far as I can tell it does almost exactly the same thing in almost exactly the same order…

Your code so far


function smallestCommons(arr) {
  arr.sort((a,b) => b-a);
  let newArr = [];
  for (let i = arr[0]; i>=arr[1]; i--) {
    newArr.push(i);
  }
  let multi = 1;
  let returnVal = 0;
  let n;

  do {
    returnVal = newArr[0] * multi;
    for (n=1; n<newArr.length; n++) {
      if (returnVal % newArr[n] !== 0) {
        multi++;
        break;
      }
    }
  } while (n !== newArr.length);
  return returnVal;
}

And it still won’t pass the last test! Apparently, the loop is still terminating before the solution is found.

I tried changing the loop to this, which would require fewer iterations:

do {
    returnVal = newArr[0] * multi * newArr[1];
    for (n=2; n<newArr.length; n++) {
      if (returnVal % newArr[n] !== 0) {
        multi++;
        break;
      }
    }
  } while (n !== newArr.length);

And still, it won’t deliver the right results for that last test! And it still works fine for all the tests with smaller numbers!

What is going on? In what way is my code functionally different from the spoiler solution?

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.77 Safari/537.36.

Link to the challenge:
https://learn.freecodecamp.org/javascript-algorithms-and-data-structures/intermediate-algorithm-scripting/smallest-common-multiple

When I run the code you wrote above, I pass all the tests.

Your first function makes 12,601 iterations between the while and for loops before returning the correct value.

The basic spoiler solution makes 631 iterations.

When I copy the 2nd do…while loop you show above, it makes your function only iterate 631 times and passes all the tests.

Well, that is super weird. When I run it, it fails the last test.

The last test passes [23,18] to the function. When I send it to the console, not only does it return the wrong answer, but it returns a different answer every time.

Could it be something about the way my browser runs the code? I tried it in Chrome and Firefox, same results.

And even so, when I paste in the spoiler solution, it works fine! Very strange.

I’ve just passed the last test with the alternative do…while loop, with the fewer iterations. I could swear that it didn’t work before, it just gave me the same strange results. But it works now. I’m still curious about why the other do…while loop doesn’t work for me.

Incidentally, here’s the very first solution I wrote. This also works for every test except the last one, but I can now see that it’s very inefficient compared to the above solutions:

function smallestCommons(arr) {
  let arrNum = arr.sort((a,b) => a-b);
  let maxNum = arrNum.pop();
  let minNum = arrNum.shift();
  let notYet = true;
  for(let i = 1; notYet; i++) {
    var multi =  maxNum * i;
    let stillNot = true;
    for (let j = minNum; j<maxNum && stillNot; j++) {
      if (multi % j != 0) {
        stillNot = false;
      }
    }
    if (stillNot) {
      notYet = false;
    }
  }
  return multi;
}

you can reuse variables
arr = arr.sort()

just use index numbers instead of methods to navigate array

min = arr[0]
max = arr[arr.length - 1]

people dont typically use booleans inside for loops.

notYet and stillNot arnt necessary.
hard to keep track of all those variables

if (something === somethingElse) {
  return true
} else {
  return false
}