Need help refactoring this code as it is producing to many iteractions

Hi guys,
anyone interested in helping me reduce the amount of interactions these lame lines of code is doing?
the last test would not pass as the fcc is stopping the runtime to protect us from running infinite loops. :slight_smile:
Here is the code:

function smallestCommons(arr) {
  //find if arr is in order.. and create range
  let range = [];
  function orderAsc() { 
    for (var i = arr[0]; i <= arr[1]; i++) {
      range.push(i);
    }
  }
  function orderDes() {
    for (var i = arr[0]; i >= arr[1]; i--) {
      range.unshift(i);
    }
  }
  (arr[0] < arr[1]) ? orderAsc() : orderDes() ; 
  
  let ans;
  let rangeNum = range.length;
  let divisor = range[rangeNum - 1];
    // checks whether an element will not return zero
  let isZero = function(element) {
    return element !== 0;
  };
  let test = [1]
  for (var i = range[rangeNum - 1]; test.some(isZero); i += divisor) {
    test = [];
    range.forEach(function(x) {
        test.push(i % x);
    });
    ans = i;
  }
  return ans;
}
smallestCommons([1, 5]); // should return 60.
smallestCommons([5, 1]); // should return 60.
smallestCommons([2, 10]); // should return 2520.
smallestCommons([1, 13]); // should return 360360.
smallestCommons([23, 18]); // should return 6056820.

What approach do you want to try next? Still on this kind, or with some math research?

Hi ieahllen,
thank you for taking the time to read my shout out.

I refactored the variable ‘divisor’ from
let divisor = range[rangeNum - 1];
to
let divisor = range.slice(-2,-1) * range.slice(-1);

just that seems to reduce the iterations enough to pass the last test.
it seems like I still need to improve on not only writing the code but also being efficient. Just tell me something, how clean was my code? easy to understand? or I could had been more clear so others can debug it?
Thank you

I see just a bunch of operations - to make it clear you should add comments

Someone says you should write your algorithm using comments (pseudocode), and then add the code to translate it to the machine

1 Like

There is a lot of stuff going on above and most of it is duplicate code. I will share some pseudo-code which would simplify what you have above.

  1. Create an empty array to store range of numbers.
  2. Assume the first element of arr is the smallest and assign its value to a min variable and assign the second element’s value to a max variable.
  3. If max < min, swap the values of min and max
  4. Iterate through the numbers min through max increasing the number by 1 each iteration and push this value to the array of numbers.

If you use variable names which describe the values they represent, you will not have to write as many comments in your code. A code reviewer will be able to make better sense of what you are attempting to do based on what you name things. For example, referencing values like arr[0], or arr[1] is hard to read, because I must go back to arr and see what those values are.

1 Like

Thanks Randell and ieahleen!
I will follow your piece of advice. As someone just starting, it’s a lot easier to my level of understanding to read someone else code when is clear, I should return the favors and do the same by sticking to this approach from early on.
cheers