Javascript, Smallest Common Multiple

this is my code for find the lowest common factor for a range of numbers. here it works well if lowest range is 1 or 2 but it does not work for other lowest range like 19,20,50 etc. but if I change the looping condition from traversing lowest range to highest range ( for(let j=arr[0];j<=arr[1];j++) ) To highest range to lowest range (for(let j=arr[1];j>=arr[0];j–))it works fine. but for my knowledge both for loop should work but it’s not. can anyone help me to figure out where I am missing

  **Your code so far**

function smallestCommons(arr) {
arr.sort((a,b) =>a-b);
let k=arr[1];

   for(let j=arr[0];j<=arr[1];j++){
        
        if(k%j!==0){
          j=arr[0];
          k=k+arr[1];
        }
        
   }
  
 console.log(k)

}


smallestCommons([6,8]);

  **Your browser information:**

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

Challenge: Smallest Common Multiple

Link to the challenge:

Your mathematical logic isn’t quite right. Can you walk me through what your code is doing and why so I can better understand?

this code comes under the Challenge:( Smallest Common Multiple) . I have linked the challenge please go through it.

I am very familiar with the challenge.

I asked you to explain what you think that your code is doing so that I can best help you. Please explain what you think that your function is doing.

it works fine if I give range of ([1,5])([1,10]) but if I give range 0f ([6,8]) it return 56 instead of 168.

It doesn’t actually work fine. You have a flaw in your logic. Please explain what you think that your code is doing.

Your loop bounds, your check for divisibility, and your update to the variable k all have problems, but it will be easier for me to explain how if I understand what you are trying to do.

let me explain the mycode-(my code expectation)
it takes the range of number as a array form the function smallestCommons(arr). then i sorted it . and give “k” to the value of arr[1] . now my code traverse trough the range of numbers from arr[0] to arr[1]. and in that if “k” is not divisible by the numbers “j”. it add arr[1] to the “k” and again start checking from the initial value of “j”. and if all the value of “j” divide the “k” then return the “k”.
Note- here I add console.log(k) for testing purpose.
Thanks.

You have the right rough idea for this general approach, but the crux of your problem is that you are incorrectly resetting j because you are hiding the fact that you actually have a nested loop.

This is problem hard to detect because you are combining the loop to generate possible multiples and the loop to check the multiplicity of the candidate. Combining these two loops makes it harder to do each task correctly.

Tweaking your code a bit for readability, what happens when you run this?

function smallestCommons(arr) {
  arr.sort((a, b) => a - b);
  let multiple = arr[1];
  for (let i = arr[0]; i <= arr[1]; i++) {
    console.log(i);  // <- this shows the problem
    if (multiple % i !== 0) {
      i = arr[0];
      multiple = multiple + arr[1];
    }
  }
  console.log(multiple);
}

smallestCommons([2, 5]);

hey thanks for helping me. but what is difference between my code and your code.
it still do not works for arr[18,23].

My code is your code with an extra console log statement to highlight the problem. What is logged when you run my code? What values of i would you expect to be logged?

I am off to bed. Personally, think the easiest way to fix this is to make nested loops. One loop to increase the multiple (your k) and one loop inside of that to check if the multiple is divisible by everything in the range.

Right now you are missing one value in the range after your first reset of the loop control valuable (accidentally not a problem with a range starting at 1 because 1 is always a divisor).

Side note, the LCM and GCD based algorithm from Wikipedia is worth learning once you have your code working. It’s a slick three line solution.

here I make changes by realizing that “i++” of the for loop increase the value of i by 1. due to this it does not starts from arr[0]. so i subtract arr[0] by 1. but it still does not work.

Actually I want to ask you that why this does not works-----

function smallestCommons(arr) {
  arr.sort((a, b) => a - b);
  let multiple = arr[1];
  for (let i = arr[0]; i <= arr[1]; i++) {
    //console.log(i);
    if (multiple % i !== 0) {
      i = arr[0];
      multiple = multiple + arr[1];
    }
  }
 return (multiple);
}

smallestCommons([2, 5]);

///////////////////////////////////////////////////////////////////////////////////////////////////////////////
But this code works by simply reversing the for loop…

function smallestCommons(arr) {
  arr.sort((a, b) => a - b);
  let multiple = arr[1];
  for (let i = arr[1]; i >= arr[0]; i--) {
    //console.log(i);
    if (multiple % i !== 0) {
      i = arr[1];
      multiple = multiple + arr[1];
    }
  }
 return (multiple);
}
smallestCommons([2, 5]);

I’ve edited your post for readability. When you enter a code block into a forum post, please precede it with a separate line of three backticks and follow it with a separate line of three backticks to make it easier to read.

You can also use the “preformatted text” tool in the editor (</>) to add backticks around text.

See this post to find the backtick on your keyboard.
Note: Backticks (`) are not single quotes (’).

The second one works because you never need to check that your multiple (k) is actually divisible by the largest number in the range. By construction it must always be divisible by arr[1].


When I add the quick hack of subtracting 1 from the reset value of i (j), it works for me.

@JeremyLT
as i understand that “i++” of for loop increase the value of “i=0” to “i=1” so
…here I make changes in “i=arr[0]” to "i=arr[0] -1 " but still it does not work. why?

Like I said… it worked for me when I did it.