Intermediate Algorithm Scripting Question

[kbventures]

Hello,

All my test are passing for this challenge(Smallest Common Multiple) except for the last one.

freeCodeCamp.org

Blockquote

function isMultiple(currentRangeNum, counterNum) {
  if (counterNum % currentRangeNum == 0) {
    return true;
  }
  return false;
}

function smallestCommons(arr) {
  let allNumbersInRangAreNotMultipletOfCounter = true;
  let counter = 1;

  let ascendingOrDescending = arr[1];
  if (arr[1] < arr[0]) {
    ascendingOrDescending = arr[0];
  }

  // Algorithm continues as long as all numbers in range not multiple of counter
  while (allNumbersInRangAreNotMultipletOfCounter) {
    allNumbersInRangAreNotMultipletOfCounter = false;
    // For Loop groups goes through range of ascending numbers ie: 1 to 5
    for (let index = 0; index < ascendingOrDescending; index++) {
      let currentRangeNumber = index + 1;

      let isMultipleResult = isMultiple(currentRangeNumber, counter);

      if (!isMultipleResult) {
        allNumbersInRangAreNotMultipletOfCounter = true;
      }
    }
    if (!allNumbersInRangAreNotMultipletOfCounter) {
      return counter;
    }
    counter++;
  }

  return counter;
}

smallestCommons([5, 1]);

module.exports = smallestCommons;

Blockquote

It doesn’t pass this test specifically:

" smallestCommons([23, 18]) should return 6056820."

[JeremyLT]

I’ve edited your post for readability. When you enter a code block into a forum post, please precede it with a separate line of three backticks and follow it with a separate line of three backticks to make it easier to read.

You can also use the “preformatted text” tool in the editor (</>) to add backticks around text.

See this post to find the backtick on your keyboard.
Note: Backticks (`) are not single quotes (’).

[JeremyLT]

Your code isn’t passing because you have efficiency problems - it is too slow for big numbers.

In order to fix this, you should

1. Start counter at a bigger value. The end result must be a multiple of one of the numbers in the range, so you should start with one of the numbers in the range.

2. Increase count by a bigger amount. You want a multiple of every number in the range, so whatever number you start with, you should only let counter be multiples of that number.

[kbventures]

Ok so changed my initial counter to range high to begin and then add in increments of range high but still not significant reduction to pass the test.

function whatIsLargestNumberInRange(num1, num2) {
  if (num1 > num2) {
    return num1;
  }
  return num2;
}

function isMultiple(currentRangeNum, counterNum) {
  if (counterNum % currentRangeNum == 0) {
    return true;
  }
  return false;
}

function smallestCommons(arr) {
  let allNumbersInRangAreNotMultipletOfCounter = true;
  let counter = whatIsLargestNumberInRange(arr[0], arr[1]);
  let rangeHigh = whatIsLargestNumberInRange(arr[0], arr[1]);

  // Algorithm continues as long as all numbers in range not multiple of counter
  while (allNumbersInRangAreNotMultipletOfCounter) {
    allNumbersInRangAreNotMultipletOfCounter = false;
    // For Loop groups goes through range of ascending numbers ie: 1 to 5
    for (let index = 0; index < rangeHigh; index++) {
      let currentRangeNumber = index + 1;

      let isMultipleResult = isMultiple(currentRangeNumber, counter);

      if (!isMultipleResult) {
        allNumbersInRangAreNotMultipletOfCounter = true;
      }
    }
    if (!allNumbersInRangAreNotMultipletOfCounter) {
      console.log(counter);
      return counter;
    }

    counter = counter + rangeHigh;
  }
}

smallestCommons([5, 1]);

module.exports = smallestCommons;

[kbventures]

My test gave me this result:

const chai = require('chai');
const expect = chai.expect;
const smallestCommons = require('../smallestCommonMultiple');

describe('Smallest Common Multiple Test Suite', () => {
  it('smallestCommons([1, 5]) should return a number.', () => {
    const result = smallestCommons([1, 5]);
    expect(result).to.be.a('number');
  });

  it('smallestCommons([1, 5]) should return 60.', () => {
    const result = smallestCommons([1, 5]);
    expect(result).to.be.eql(60);
  });

  it('smallestCommons([5, 1]) should return 60.', () => {
    const result = smallestCommons([5, 1]);
    expect(result).to.be.eql(60);
  });

  it('smallestCommons([2, 10]) should return 2520.', () => {
    const result = smallestCommons([2, 10]);
    expect(result).to.be.eql(2520);
  });

  it('smallestCommons([1, 13]) should return 360360.', () => {
    const result = smallestCommons([1, 13]);
    expect(result).to.be.eql(360360);
  });

  it('smallestCommons([23, 18]) should return 6056820.', () => {
    const result = smallestCommons([23, 18]);
    expect(result).to.be.eql(6056820);
  });
});

1 failing

1. Smallest Common Multiple Test Suite
   smallestCommons([23, 18]) should return 6056820.:

   AssertionError: expected 5354228880 to deeply equal 6056820

   • expected - actual

   -5354228880
   +6056820

   at Context. (test/test.smallestCommonMutiples.js:33:26)
   at processImmediate (internal/timers.js:461:21)

type or paste code here

[JeremyLT]

@kbventures

for (let index = 0; index < rangeHigh; index++) {

These loop bounds are not quite right. Why start at index 0?

[kbventures]

I was able to fix it. am confused has to why most of the test still passed but yeah it makes no sense for index to not at least start at range low. Once the algorithm picked up speed(efficiency) the tests in mocha chai had an incorrect response. Thanks for the assistance.

function findRangeLow(num1, num2) {
  if (num1 < num2) {
    return num1;
  }
  return num2;
}

function whatIsLargestNumberInRange(num1, num2) {
  if (num1 > num2) {
    return num1;
  }
  return num2;
}

function isMultiple(currentRangeNum, counterNum) {
  if (counterNum % currentRangeNum == 0) {
    return true;
  }
  return false;
}

function smallestCommons(arr) {
  let allNumbersInRangAreNotMultipletOfCounter = true;
  let counter = whatIsLargestNumberInRange(arr[0], arr[1]);
  let rangeHigh = whatIsLargestNumberInRange(arr[0], arr[1]);
  let rangeLow = findRangeLow(arr[0], arr[1]);

  // Algorithm continues as long as all numbers in range not multiple of counter
  while (allNumbersInRangAreNotMultipletOfCounter) {
    allNumbersInRangAreNotMultipletOfCounter = false;
    // For Loop groups goes through range of ascending numbers ie: 1 to 5
    for (let index = rangeLow; index <= rangeHigh; index++) {
      let currentRangeNumber = index;
      // console.log(index);
      // console.log(rangeHigh);
      // console.log(currentRangeNumber);
      let isMultipleResult = isMultiple(currentRangeNumber, counter);

      if (!isMultipleResult) {
        allNumbersInRangAreNotMultipletOfCounter = true;
      }
    }
    if (!allNumbersInRangAreNotMultipletOfCounter) {
      console.log(counter);
      return counter;
    }
    // rangeLow = rangeLow + rangeLow;
    counter = counter + rangeHigh;
  }
}

smallestCommons([5, 1]);

module.exports = smallestCommons;

[JeremyLT]

If you want to see a faster, tidier solution, I would look at the LCM + GCD approach. The formulae are on Wikipedia.