Smallest Common Multiple - help, I'm stuck!

Hi everyone!

I’m currently stuck on the “Smallest Common Multiple” task and I feel I made things way to complicated, but also have no solid idea what I could do differently or what to google for to get closer to the solution. Also I don’t want to give up and just read the solution, so I decided to ask for help here.

This is my code (I tried to explain what I intended to do in the code directly rather then explaining it beforehand). It would be much appreciated if anyone can explain me what is going wrong and why it’s going wrong and whether or not I’m on the right track or I should rather start over (if that is the case, some pointers for what to do different in the next attempt would be much appreciated). Thanks in advance!

function smallestCommons(arr) {
  
  var orderedArr = arr;
  var fullNumArr = [];
  var compareArr = [];
  
  //orders arr in orderedArr, so that higher number is always the first one
  if (arr[0] > arr[1]){
    orderedArr = [arr[1], arr[0]];
  }
  
  //prepares arrays needed in findNum function
  function prep (value){ 
    //creates an array with all numbers between arr[0] and arr[1] (from largest to smallest number)
    //creates subarrays for compareArr, that will be filled up in the next for loop
    for (var i = orderedArr[1]; i >= 1; i--){
      if (i >= orderedArr[0] && i <= orderedArr[1]){
        fullNumArr.push(i);
        compareArr.push([]);
      } else {
        return compareArr;
      }
    }    
    //fills subarrays of compareArr with multiples of each number, so that they can be compared within the findNum function
    for (var j = 0; j < fullNumArr.length; j++){
      for (var k = 1; k <= value; k++){
        compareArr[j].push(fullNumArr[j] * k);
      }
    }
    return compareArr;
  }

 //findNum compares compareArr[0] and compareArr[1] to find the smallest common multiple of these 2
 //then it replaces the first two numbers in the fullNumArr with this smallest common multiple
 //then it should (but currently doens't) go through the prep function again to update compareArr
 //based on the changes made to fullNumArr. I want to repeat this process until only 1 number is left
 //in the fullNumArr array and then (in my theory) the only thing left to do would be to return that number
 function findNum (){
    prep(orderedArr[1]);
    for (var l = 0; l < compareArr.length; l++){
      if (compareArr[l].length > 1){
        for (var m = 0; m < compareArr[l].length; m++){
          for (var n = 0; n < compareArr[l+1].length; n++){
            if (compareArr.length == 1){
              return fullNumArr[0];
            }
            else if (compareArr[l][m] == compareArr[l+1][n]){
              fullNumArr[l] = compareArr[l][m];
              fullNumArr.splice(l+1, 1);
              compareArr = [];
              prep(fullNumArr[0] * fullNumArr[1]);
            }
          }
        }
      } else {
      return fullNumArr[0];
      }      
    }
  }
  
  return findNum();
}


smallestCommons([1, 5]);

I’ve edited your post for readability. When you enter a code block into the forum, precede it with a line of three backticks and follow it with a line of three backticks to make easier to read. See this post to find the backtick on your keyboard. The “preformatted text” tool in the editor (</>) will also add backticks around text.

markdown_Forums

Currently, your code has an error ( TypeError: Cannot read property ‘push’ of undefined) caused by the line I have marked (see below) with a comment in following section of code.

    for (var j = 0; j < fullNumArr.length; j++) {
      for (var k = 1; k <= value; k++) {
        compareArr[j].push(fullNumArr[j] * k); // at some point in your code, compare[j] is undefined, which caused the error message I mentioned above
      }
    }

Fixing this error may or may not allow you to solve the challenge, but you may get closer to a solution with it resolved.

Also, did you write out an algorithm in plain language first before trying to write any code? If not, you should do that first, to make sure you can walk through an example test and make sure your written steps would produce a correct result.

While you have have put several comments through out your code of what a section of code does, you have not always explained why you wrote specific sections of code. It is not clear to me why you are approaching this challenge in the manner you are. A quick search on google pulls up a wiki that describes several methods of calculating the least common multiple, which is what this challenge is all about. If you can not figure out your current solution, then I would review some of the algorithm’s discussed in this wiki and try to adapt one of them to this solution.

Thanks for the editing and the tips along the way. I haven’t written the algorithm out in plain language, but I will try that out in the future. I think I understood what the code should do in theory (the link in included in the task description helped a lot with this), but I couldn’t figure out yet, how to write the code in order to achieve just that (apart from the solution I tried to pursue).

Sorry to hear, that my explanations weren’t sufficient. I tried to explain my thought process as well as possible. I appreciate your help, but if I’m honest, I’m still pretty lost and unsure how to proceed in order to get closer to the solution.

function smallestCommons(arr) {
  let sa = arr.sort((a,b)=>a-b);
  let sa1 = []
for (let i=sa[0];i<=sa[1];i++){
  sa1.push(i);
};
let mult = sa1.reduce((a,b)=>a*b,1);
for (let j=sa[1];j<=mult;j++){
 if (sa1.every(x => j %x ==0)){
   return j;
 }
 }
 }

This code works on console but does not pass the last two tests. Why?

The first problem is that your code is way too complicated. (I gave up on reading by looking at the structure, I haven’t even tried frankly) No offense but it’s a waste of time to modify current code to make it work because there are too many lucking errors.

This problem is happening because you are packing everything this function does in one place. In fact, you can break up the problem and have function for each of sub-problem.

I see at least three separate things.

  1. Expanding the given range argument. e.g) [5, 8] => [5, 6, 7, 8]
  2. Computing the smallest common multiple of two numbers. (**most important part)
  3. Computing the smallest common multiple of many numbers based on #2.

If you can correctly implement these ideas, the resulting answer will look much better and identifying any error will be much simpler.

So, take your time and try to implement these things piece by piece. Could you do that?

If your code produces correct answer for every test on your environment, but fails on here or fails for a large input, then it is a common problem and you can move on if you wish to.

You can read about the similar discussion from this post and on. It won’t give you the exact reason, but you will know this is a common problem for challenges of this type.

Yeah. It happened to me here 1 day ago:

I don’t know about the details but I think that this is a common issue in freecodecamp where some codes that requires some loops won’t work. It works in the console but it just doesn’t pass the tests for unknown reasons. Maybe we should report about this issue or something.

It is because Free Code Camp has a feature to protect against code with infinite loops. It is triggered when the code takes to long to process. Once it activates, the code just stops resulting in test results not being correct which would work given enough time.

All of the challenges here on FCC can be solved with an algorithm efficient enough to avoid triggering the infinite loop protection. Sometimes, an algorithm being used by a camper only triggers the protection on a particular case such as the last test case for the Smallest Common Multiple.

1 Like