# Code works for all but the last test

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;
const a2 = a
let b = arr;
const b2 = b
let list = [];

if (a < b) {
for (let i = a; i <= b; i++) {
list.push(i);
}
for (let i = list; i < b; i++) {
if ((a * b) % list[i] !== 0) {
a = a + a2
i = list
}
}
return a * b
} else {
for (let i = b; i <= a; i++) {
list.push(i)
}
for (let i = list; i < list.length; i++) {
if ((b * a) % list[i] !== 0) {
b = b + b2
i = list
}
}
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`

Challenge: Smallest Common Multiple

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])`.

1 Like

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.

2 Likes

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.

1 Like

I would suggest you use better variable names

oh absolutely, my bad. i was in a rush to post and forgot.

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.

Right, I think that’s happening because the approach your are using has fundamental mathematical flaws.

1 Like

ok. thank your for the input. i will learn about the proper algorithms to use and then come back to the exercise. 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;
const a2 = a
let b = arr;
const b2 = b
let list = [];

if (a < b) {
for (let i = a; i <= b; i++) {
list.push(i);
}
for (let i = list; i < b; i++) {
if ((a * b) % i !== 0) {
a = a + a2
i = list
}
}
return a * b
} else {
for (let i = b; i <= a; i++) {
list.push(i)
}
for (let i = list; i < a; i++) {
if ((b * a) % i !== 0) {
b = b + b2
i = list
}
}
return a * b
}
}

console.log(smallestCommons([23, 18]));
``````

Strange, I had tried this, but it did not seem to work.

Side note: this is a huge anti-pattern and shouldn’t really be done in production code.

why is that? do you have a reference on anti-pattern? im not sure what that is

An anti-pattern is basically something that works but ends up causing more trouble than it solves.

In this case, your code falls afoul of

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’:

1 Like

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.

You can also get rid of the unnecessary `list` variable. Removing the looping back is trickier.