Smallest Common Multiple - higher number

Tell us what’s happening:

This code is working fine for the smaller numbers, as it grows it does not provide desired result.

In this example,
smallestCommons([1, 5]) - working fine
smallestCommons([5, 1]) - working fine
smallestCommons([2, 10]) - working fine
smallestCommons([1, 13]) - not getting the desired result
smallestCommons([23, 18]) - not getting the desired result

please help me understand what I’m missing here, i dont think there is an error in logic, but I miss something which I couldnt figure it out, please help.

Your code so far


function smallestCommons(arr) {
  var valueArr = [];
  var divisor = 1;
  var divisorArr = [];
  var scm;

  arr.sort((a, b) => a - b);

  for(var i = arr[0]; i <= arr[1]; i++) {
    valueArr.push(i);
  }

  while(!scm) {
    for(var j = 0; j < valueArr.length; j++) {
      if(divisor % valueArr[j] == 0) {
        divisorArr.push(divisor);
      }
    }

    if(divisorArr.length == valueArr.length) {
      scm = divisor;
    } else {
      divisorArr = [];
      divisor++;
    }
  }

  return scm;
}

smallestCommons([1,5]);

Your browser information:

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

Link to the challenge:

As a rule of thumb, when you fail the last couple challenges, that usually means that the algorithm is not efficient enough. I confirmed this because when I run your code elsewhere, it gives the right results. But the FCC challenge has a timer on it so it will timeout if it is taking too long - on purpose.

Yes, these challenges are tough. They are challenging. But this kind of thinking will be invaluable on job interviews where they often give you challenges like this. Being able to turn the problem over in your head and think of a couple different solutions and have a sense for which is most efficient - this is an invaluable skill.

Just keep at it. See if you can think of ways to make it more efficient.

1 Like

Your code is not efficient enough.
It triggers the infine loop protection

Find an other way that doesn’t consist checking each number till the hundred thousands range

thanks that helps and understood the problem, its not about getting correct logic but also to get efficient logic.

Well, obviously getting the correct logic is good. But in an interview, they might say, “OK, that works but I think there is a more efficient way. Why don’t you spend some time looking for it.”

In an interview, I might quickly find a “brute force” solution that solve the problem and then say, “OK, I think I can improve on this …”

These kinds of things matter. Imagine if you were processing billions of pieces of data. I was recently working on an algorithms and came up with a solution. When I ran it, the laptop revved up to the point where I thought it was going to explode. It stayed like that for 6 hours before I gave up. I knew the algorithm worked because I’d tried it with smaller numbers. So the next day I rethought the whole thing and ran a new algorithm that took less than a second, for the same large numbers. These things can make a big difference.

Thanks Kevin, I found an alternative solution and it worked in FCC :slight_smile: , below is my code. Please share your feedback.

function smallestCommons(arr) {
  
  var multiplier = 2;
  var divisorArr = [];
  var tempArr = [];
  var scm;

  arr.sort((a, b) => a - b);

  for(var i = arr[0]; i <= arr[1]; i++) {
    tempArr.push(i);
  }

  while(divisorArr.every((divisor) => divisor !== 1)) {
    if (tempArr.some((value) => value % multiplier == 0)) {
      divisorArr.push(multiplier);
      tempArr = tempArr.map(function(tempValue) {
        if(tempValue % multiplier == 0) {
          return tempValue / multiplier;
        } else {
          return tempValue;
        }
        console.log(tempArr);
      });
    } else {
      multiplier++;
    }

  }

  scm = divisorArr.reduce((total, value) => total * value);

  return scm;
}

smallestCommons([1,5]);

Please don’t post solutions to curriculum without blurring. To do that, put spoiler tags ([spoiler] and [/spoiler]) on the line before and after. And, if you put three backticks on the line before your code and three more on the line after, then the code will format nicely.

1 Like

Thanks Kevin, appreciate your support.

With an excitement I just pasted the solution that worked, apologise for that, from next time will do it as you suggested.

Cool, it happens. Glad you solved it. Good luck on the next one. Don’t get discouraged - some of these are hard.

1 Like