Help! Smallest Common Multiple Algorithm--Passing all tests but the final one


#1

I can’t figure out what’s wrong with my code. It passes ALL tests except for the final one which requires me to find the smallest common multiple of 23 and 18 given the requirements. Here is my code. Yes, it’s definitely not the best code, I was planning to clean it up later once I got the basic requirements down.

    <code>
    function smallestCommons(arr) {
      //sorts parameters
      var smaller;
      var bigger;
      if (arr[0] < arr[1]) {
        smaller = arr[0];
        bigger = arr[1];
      }
      else {
        smaller = arr[1];
        bigger = arr[0];
      }
      
      //creates array for smallest common multiples of both parameters
      var multiples_bigger = [];
      var multiples_smaller = [];
      
      //finds smallest common multiple that meets requirements for each parameter
      for (var j = 1; j < 10000000; j++) {
        if ((bigger * j) % 2 == 0 && (smaller * j) % 2 == 0) {
          for (var q = smaller; q < bigger; q++) {
            if ((bigger * j) % q != 0 && (smaller * j) % q != 0) {
               break;
            }
            else if (q == bigger - 1) {
              multiples_bigger.push(bigger * j);
              multiples_smaller.push(smaller * j);
            }
          }
        }  
      }

      //placeholder number that will be used to test value of numbers in array
      var smaller_number = 10000000;
      
      //finds the smallest common multiple in the big array
      for (var z = 0; z < multiples_bigger.length; z++) {
        if (multiples_bigger[z] <= smaller_number) {
          smaller_number = multiples_bigger[z];
        }
      }
      //matches smallest common multiple in big array with element in small array
      var lcm = multiples_smaller.indexOf(smaller_number);
      
      
      return multiples_smaller[lcm];
    }


    smallestCommons([23, 18]);

    </code>

#2

First of all, please enclose your code in three backticks (below the ESC key), on the lines before and after to make it look good

:

As to your code, the first problem I see is the use of a “magic number”, 10000000. There is something wrong with your algorithm if you need to do that.

Don’t get frustrated - this is one of the challenges with which people commonly have problems.

Sorry, I don’t have time at the moment to go through and explain it. Perhaps tomorrow if no one else has a chance. Take some time and rethink what you want to do.


#3

OK, I felt a little guilty for not giving a better answer.

Think of the problem this way. You need to find the lowest common multiple of several numbers. So I start by finding the lowest and highest number in that range. Now, you know that whatever this lcm is, it must be a multiple of the largest number in the range (let’s call that max).

So I would start with lcm set to the largest number in the range, then I would loop to see if all the other numbers can evenly divide into it. As soon as any of those numbers can’t divide into lcm, then I know I can break out of the loop and try the next candidate lcm. And I just keep adding max to my previous candidate lcm - remember that our final lcm must be a multiple of max (and all the other numbers, but max is the biggest so let’s index by that.) If you ever get through your whole range of numbers from min to max-1 (no need to check max since we’re indexing by it) then we know we’ve found our lcm.

I hope that helps without giving too much away.


#4

Hi, thank you for your response. I’m still a little confused, might just clear out all my code and rewrite with another method in mind.


#5

Yeah, there were a couple of these that I had to start from scratch a few times. This might be one of them. I did my work in codepen because it was easier than doing it in the fCC window.

Was there something specific that is still confusing?