can anybody tell me why this code works for each test except the last one?
it succesfully works for [1, 5], [2, 10], and [1, 13], but fails on [18, 23].
ive been trying to fix it for 6+ hours and i cannot figure it out. ive console logged each variable at each iteration and they all seem to be correct.
when a = 263340, b = 23, and i =19 it still passes the if statement on line 13 and i have no idea why.
**Your code so far**
function smallestCommons(arr) {
let a = arr[0];
const a2 = a
let b = arr[1];
const b2 = b
let list = [];
if (a < b) {
for (let i = a; i <= b; i++) {
list.push(i);
}
for (let i = list[0]; i < b; i++) {
if ((a * b) % list[i] !== 0) {
a = a + a2
i = list[0]
}
}
return a * b
} else {
for (let i = b; i <= a; i++) {
list.push(i)
}
for (let i = list[0]; i < list.length; i++) {
if ((b * a) % list[i] !== 0) {
b = b + b2
i = list[0]
}
}
return a * b
}
}
console.log(smallestCommons([1, 5]));
**Your browser information:**
User Agent is: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/94.0.4606.61 Safari/537.36
I think that your algorithm is not mathematically sound, unfortunately. It is not just the last test; your code also fails for me when I use smallestCommons([2, 10]).
I would suggest you use better variable names to make the code easier for everyone to follow. Also, don’t use if/else to handle the issue of which arg is bigger because you are basically just duplicating code. Set your min and max values at the very beginning and then you have just one block of code to deal with.
A lot of people get stuck on this challenge. Coding it is not the hardest part. Understanding how the algorithm works is. Once you understand the algorithm then the code is really not that complicated. I would suggest you make sure you understand exactly what you need to do to find the SCM. You can definitely ask questions about that here too.
I totally agree with this. I highly recommend researching what algorithm to use on Wikipedia and code up the algorithm(s) described there to find the LCM of multiple numbers.
I understand what youre saying, and i will happily do that.
however,
i can see the correct answer come up in the console, but the if statement doesnt catch it. it just continues to iterate past that point. that is what im confused about specifically.
This is one where the math that is involved is usually Sophomore level in college - it can be tricky to build from scratch if you don’t have a math background.
hey i actually got my code to work. my if statment was pulling list[i] instead of just using i.
i should still spend some time learning more about algorithms though. haha
function smallestCommons(arr) {
let a = arr[0];
const a2 = a
let b = arr[1];
const b2 = b
let list = [];
if (a < b) {
for (let i = a; i <= b; i++) {
list.push(i);
}
for (let i = list[0]; i < b; i++) {
if ((a * b) % i !== 0) {
a = a + a2
i = list[0]
}
}
return a * b
} else {
for (let i = b; i <= a; i++) {
list.push(i)
}
for (let i = list[0]; i < a; i++) {
if ((b * a) % i !== 0) {
b = b + b2
i = list[0]
}
}
return a * b
}
}
console.log(smallestCommons([23, 18]));
We’ve all written code that falls afoul of various anti-patterns.
Its a little hard to specifically pin down exact anti-patterns. Sometimes its easier to explain as an anti-pattern though instead of talking about ‘code smell’:
If you set the min/max values at the very beginning and get rid of the if/else then you’ll solve most of these issues. You can set the min/max variables with one line of code if you are tricky enough.
I set up an if statement at the beginning to simplify the code.
it assigns a and b accordingly at the beginning so I don’t have to rewrite the code block twice.
as for removing the ‘for loop reset’, I was thinking I could turn it into a function and then call it multiple times in a while statement. I haven’t actually tried that yet though.
