Freecodecamp: Inconsistent results for 'smallestCommons([23, 18])'

[thoragio]

Challenge Smallest Common Multiple has an issue.
User Agent is: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.82 Safari/537.36.
Please describe how to reproduce this issue, and include links to screenshots if possible.

I've tested my code using both Node 6.1.0 on my local machine and in CodePen and it returns the correct results (check the browser console log):
https://codepen.io/thoragio/pen/LkgKjA

However, when I run this same code against the test in the challenge, the last test ( smallestCommons([23, 18]) ) fails. When I look at the browser console log, I see that my return value is always a different number somewhere in the 4 million range (e.g., 4841427, 4701970, 4950014), but the correct answer is supposed to be 6056820.

My code:

"use strict"

const smallestCommons = (arr) => {

  let range = []
  let rangeSize = arr[1] > arr[0] ? arr[1] - arr[0] + 1 : arr[0] - arr[1] + 1
  let rangeStart = arr[1] > arr[0] ? arr[0] : arr[1]
  for (let i = 0; i < rangeSize; i++) {
    range.push(rangeStart)
    rangeStart++
  }
  console.log(range)

  let checkSmallest = range[range.length - 1]
  console.log(checkSmallest)

  for (let j = 0; j < range.length; j++) {
    if (checkSmallest % range[j] == 0) {
    }
    else {
      j = -1
      checkSmallest++
    }
  }
  console.log(checkSmallest)
  return checkSmallest
}

smallestCommons([1,5]);

[erictleung]

@thoragio running your code gives this console output

Error: Potential infinite loop at line 18. To disable loop protection, write: 
// noprotect
as the first line. Beware that if you do have an infinite loop in your code this will crash your browser.

so you could add the // no protect to see it works.

[alistermada]

I just tried the code above with // no protect. I also get inconsistent results for smallestCommons([23, 18]). However, instead of getting numbers in the 4 millions I get number in the 1 millions like 1902775, 1892104, and 1915324. The other tests are consistently correct. Tested on Chrome.

On the latest Firefox, smallestCommons([1, 13]) is also giving inconsistent results.

[BKinahan]

This code is technically correct in that it outputs the correct values given sufficient time, but it is quite inefficient and inconsistent results in the FCC environment are most likely due to this inefficiency.

For example, incrementing checkSmallest by 1 on each step through the loop when it is not divisible by one of the numbers in the range will make the code take about twenty times longer for the input [23,18] than incrementing by the larger of the two input numbers. (Because the number we are looking for as the smallest common multiple must be a multiple of 23, incrementing by 23 rather than by 1 saves a lot of time.)

That simple change allows this code to pass all tests for this challenge. There are other inefficiencies but this is not the place for further discussion of those.

[thoragio]

@erictleung @alistermada @BKinahan Thanks for looking into this.

I appreciate that my code is pretty inefficient, but in my early stages of learning JS I am more focused on "getting it to work" than "getting it to work optimally". Given that, I am not sure if what I have observed is really worthy of being deemed a bug to fix in the FCC platform, but it was confusing for me because my code worked fine running with Node locally and on another hosted platform (CodePen).

I will leave it up to you guys whether you want to keep this issue open and address it somehow (perhaps just a mention somewhere in the challenge description that inefficient code might fail some of the tests?) or just close it. Thanks for your time considering it in any case.

And thank you for the great work on FCC. I really appreciate this great learning resource.

[raisedadead]

@thoragio thanks a lot for the appreciation, and while I agree that your code is correct and I 100% support the fact that as a beginner you are focused on "getting it to work"! Goodjob!

However, to incorporate a corner case like this, we would have to probably brainstorm and churn up a logic that can handle the inconsistent errors like this.

Given the priorities I think this may not be taken up given the complexity it involves. Not to forget that every browser behaves differently.

address it somehow (perhaps just a mention somewhere in the challenge description that inefficient code might fail some of the tests?)

Yes, we can mention that some code might break, but then that would have to do it with all challenges.

Leaving it open for other @FreeCodeCamp/issue-moderators to comment. But I think this can be closed as a wontfix.

[raisedadead]

Closing as this involves a complex approach than necessary.

[RandellDawson]

@carlotapia This is not the place to ask coding questions related to challenges. Please use our forum for such questions. Thank you.