I’m having issues understanding what’s going wrong with the final test ([23,18]).
The code shouldn’t stop until “result” is divizible with all numbers ([ 18, 19, 20, 21, 22, 23 ]) and smaller than the biggest possible result (upperLimit) but when it gets to 2018940 it just stops by itself, despite divizible(18,2018940)==false.
I have alternatives that do work, I’m just curious why this doesn’t so I can avoid doing something wrong later. Why is it stopping short, despite neither conditions being fulfilled (2018940 is smaller than the upperLimit variable, and is not divizible by 18)
Thank you.
Your code so far
function smallestCommons(arr) {
let newArr=arr;
if(arr[0]>arr[1])
newArr=arr.toSorted()
// sort if necessary
let upperLimit=1;
// define the upper limit of what result can be so it doesn't register an infinitet loop
let individualArr=[];
// create array with every number between arr[0] and arr[1]
let result=newArr[1];
// define result as the largest between the two elements
let divizible=(x,y)=>y%x==0;
// define a function to test divizibility of two elements
for(let i=newArr[0];i<=newArr[1];i++)
{individualArr.push(i);
upperLimit=upperLimit*i;
}
// actually pushing the elements to create the individual numbers array, and calculate the upper limit at the same time
console.log(individualArr);
// test to see if it did the job
for (let i=0;i<individualArr.length;i++)
if(!divizible(individualArr[i],result)&&result<=upperLimit)
{result+=1;i=0}
// go through the individual number array
// ex: [23,18] would give [18,19,20,21,22,23]
// test if the result is divizible with each one; if not, add 1 to the result variable, reset i to start testing again until result is divizible with all numbers
return result;
}
Your browser information:
User Agent is: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/114.0.0.0 Safari/537.36
Challenge: Intermediate Algorithm Scripting - Smallest Common Multiple
if perfomance is the issue here, you can speed this code up by changing the step here:
you do not need to check every natural number
and also - you defined upper limit -which is factorial - you can also think what would be the lower limit for result and how they relate to each other
yeah, I’m applying various solutions to the same problem, trying to learn how things work better by doing that (like now running into the infinite loop protection).
Generally, I write the code sloppy just to get the result I neeed, then streamline wherever I can. This one got way more complicated than I wanted it because I’ve been trying to force the solution.
Tried addressing that with comments, I know it’s not the prettiest rn.
I’ve got this: {result=result+newArr[1];i=0}
So only checking multiples of the highest number (23 in my case).
Bit tired, can’t quite figure out what you meant with the rest of the advice, but thank you regardless.
I can give a further hint I think.
lets take a case for [1, 5], for example
so upperlimit for this one would be 5!, which is 120
if factorize 120 >>> 120 = 2 * 2 * 2 * 3 * 5
I claim that lower limit for this case would be 30( multiplication of prime factors of 120 >>> 2 * 3* 5 = 30)
and for finding result here one can use step 30 also
so we need to check numbers 30, 60, 90, 120
so with using 30 as lower limit, and also 30 as a step for loop you would have 4 iterations max for [1,5]
in fact, there would 2 iterations total, since the answer is 60 for this case
the downside is - you would need to add code for calculting lower limit
factorization will add some aditional computation of course but it will also reduce number of iterations in this loop dramatically:
That would be one way to do it, but my current issue is I’m not even getting to the upperLimit before the code stops.
I’m looking at Solution 1 on the forum:
function smallestCommons(arr) {
// Setup
const [min, max] = arr.sort((a, b) => a - b);
const numberDivisors = max - min + 1;
// Largest possible value for SCM
let upperBound = 1;
for (let i = min; i <= max; i++) {
upperBound *= i;
}
// Test all multiples of 'max'
for (let multiple = max; multiple <= upperBound; multiple += max) {
// Check if every value in range divides 'multiple'
let divisorCount = 0;
for (let i = min; i <= max; i++) {
// Count divisors
if (multiple % i === 0) {
divisorCount += 1;
}
}
if (divisorCount === numberDivisors) {
return multiple;
}
}
}
smallestCommons([1, 5]);
At a glance, they seem to be using the same technique I’m using, and I can’t quite figure out what they’re doing different.
They sort it, set an upperBound (my upperLimit), then go through every multiple of the largest number.
Why does my code break, but theirs doesn’t?
