Smallest Common Multiple - checks out but won't pass

JavaScript
1

I know this is ugly… it took me about 3 hours and after reading some solutions on here I threw up in my mouth a few times. ANYWAY, it does work, but it won’t let me pass the challenge. smallestCommons([1, 5]) should return 60. (it does) , smallestCommons([5, 1]) should return 60. (it does!), smallestCommons([23, 18]) should return 6056820. (it does!!).

After watching some youtube math stuff about finding LCM from a set (multiply the common factors (ignoring duplicates.)) I went about it this way:

// global object for greatest occurence of each prime number
let greatestPrimes = {};

function smallestCommons(arr) {
  // fill newArr with all nums in range
  let newArr = arr.sort((a, b) => a - b);
  let min = newArr[0];
  let max = newArr[1];
  for (min += 1; min < max; min++) {
    newArr.push(min);
  }

  // Get common factors
  for (let i = 0; i < newArr.length; i++) {
// see below    
primeFactors(newArr[i]);
  }

  // multiply common factors for LCM
  let lcm = 1;
  for (let key in greatestPrimes) {
    let factor = parseInt(key);
    lcm *= Math.pow(factor, greatestPrimes[key]);
  }
  console.log(lcm);
  return lcm;
}

and here is how I get the common factors, and then track the greatest occurrence of each for use as exponent:

// find prime factors, adapted from Alan's answer to find prime factors of a number: 
// https://stackoverflow.com/questions/39899072/how-to-find-prime-factors-using-a-for-loop-in-javascript
function primeFactors(n){
  let factors = [];
  let divisor = 2;
  if (n === 1) {factors.push(1)};
  while(n >= 2){
    if(n % divisor === 0){
       factors.push(divisor); 
       n = n / divisor;
    } else {
      divisor++;
    }     
  }

  // from w3resource.com - greatest occurence of item in array
  const countOccurrences = (array, val) => array.reduce((a, v) => (v === val ? a + 1 : a), 0);
 
    for (let i = 0; i < factors.length; i++) {
        let key = factors[i];
        let count = countOccurrences(factors, factors[i]);
        // if it's new, or if it's the greatest, add to greatestPrimes global object
        if (count > greatestPrimes[key] || !greatestPrimes.hasOwnProperty(key)) {
          greatestPrimes[key] = count;
        }
    }
}

This checks out for me with each test provided by the challenge. What am I missing ?(besides the code being 50 lines longer than it needs to be!)

2

Your code contains global variables that are changed each time the function is run. This means that after each test completes, subsequent tests start with the previous value. To fix this, make sure your function doesn’t change any global variables, and declare/assign variables within the function if they need to be changed.

Example:

var myGlobal = [1];
function returnGlobal(arg) {
  myGlobal.push(arg);
  return myGlobal;
} // unreliable - array gets longer each time the function is run

function returnLocal(arg) {
  var myLocal = [1];
  myLocal.push(arg);
  return myLocal;
} // reliable - always returns an array of length 2
3

but my global variable is supposed to get longer each time if it needs to add a new occurrence of a factor not seen before.

4

but what happens if you call smallestCommon multiple times?

like

console.log(smallestCommon([1,6]))
console.log(smallestCommon([1,3]))

does the second function give the expected value?

1 Like
5

aha! thank you! So I had to initialize the global before returning smallestCommon.

6

you should not use a global variable

but if you are confortable with leaving it like this…

2 Likes
7

There is no need to have that variable outside of your function. It is always best to reduce variables to the absolute smallest scope you can.