Skip to main content
removed stuff
Source Link

Formatting

Before addressing optimizations, let's address some formatting problems

while vs. for loops

It is preferred to use a for loop when you:

  1. Have an incrementer
  2. The incrementer is not used outside of the loop
  3. The incrementer is either strictly increasing or strictly decreasing

This is the case with both of your while loops. They can be refactored to the following:

In listFactors():

for (var i = 2; i <= number; i++){
    if(isFactor(number, i)){
        factors.unshift(i);
    }
}

In GCF():

for (var count = 0; count < factorsOfFirst.length; count++){
    var toTest = factorsOfFirst[count];
    var passTest = factorsOfEach.every(arrayContains);
    if (passTest) {
        return toTest;
    }
}

Avoid explicit booleans when possible

Instead of:

if(num % fact === 0){
    return true;
}
else{
    return false;
}

it is preferable to write:

return num % fact === 0;

and the same for your other functions.

Avoid global variables

As you correctly pointed out, you should try not to use mutable global variables; it makes the code much harder to understand. To avoid this, you could make the arrayContains function merely an arrow function and pass in toTest as well:

var testPassed = factorsOfEach.every(arr => arr.indexOf(toTest) !== -1);

A Cleaner Solution

Using the Euclidean Algorithm to find the GCD/GCF of two numbers (assuming non-negative numbers) is a much more cleaner and optimized approach:

function GCF(a, b) {
    if (b === 0) return a;
    if (a < b)  return GCF(b, a);
    else         return GCF(b, a % b);
}

Then, just use a reduce statement to apply GCF to all of the numbers in the array:

function findGCFofList(list) {
    return list.reduce(GCF);
}

    

Formatting

Before addressing optimizations, let's address some formatting problems

while vs. for loops

It is preferred to use a for loop when you:

  1. Have an incrementer
  2. The incrementer is not used outside of the loop
  3. The incrementer is either strictly increasing or strictly decreasing

This is the case with both of your while loops. They can be refactored to the following:

In listFactors():

for (var i = 2; i <= number; i++){
    if(isFactor(number, i)){
        factors.unshift(i);
    }
}

In GCF():

for (var count = 0; count < factorsOfFirst.length; count++){
    var toTest = factorsOfFirst[count];
    var passTest = factorsOfEach.every(arrayContains);
    if (passTest) {
        return toTest;
    }
}

Avoid explicit booleans when possible

Instead of:

if(num % fact === 0){
    return true;
}
else{
    return false;
}

it is preferable to write:

return num % fact === 0;

and the same for your other functions.

Avoid global variables

As you correctly pointed out, you should try not to use mutable global variables; it makes the code much harder to understand. To avoid this, you could make the arrayContains function merely an arrow function and pass in toTest as well:

var testPassed = factorsOfEach.every(arr => arr.indexOf(toTest) !== -1);

A Cleaner Solution

Using the Euclidean Algorithm to find the GCD/GCF of two numbers (assuming non-negative numbers) is a much more cleaner and optimized approach:

function GCF(a, b) {
    if (b === 0) return a;
    if (a < b)  return GCF(b, a);
    else        return GCF(b, a % b);
}

Then, just use a reduce statement to apply GCF to all of the numbers in the array:

function findGCFofList(list) {
    return list.reduce(GCF);
}

    

Formatting

Before addressing optimizations, let's address some formatting problems

while vs. for loops

It is preferred to use a for loop when you:

  1. Have an incrementer
  2. The incrementer is not used outside of the loop
  3. The incrementer is either strictly increasing or strictly decreasing

This is the case with both of your while loops. They can be refactored to the following:

In listFactors():

for (var i = 2; i <= number; i++){
    if(isFactor(number, i)){
        factors.unshift(i);
    }
}

In GCF():

for (var count = 0; count < factorsOfFirst.length; count++){
    var toTest = factorsOfFirst[count];
    var passTest = factorsOfEach.every(arrayContains);
    if (passTest) {
        return toTest;
    }
}

Avoid explicit booleans when possible

Instead of:

if(num % fact === 0){
    return true;
}
else{
    return false;
}

it is preferable to write:

return num % fact === 0;

and the same for your other functions.

Avoid global variables

As you correctly pointed out, you should try not to use mutable global variables; it makes the code much harder to understand. To avoid this, you could make the arrayContains function merely an arrow function and pass in toTest as well:

var testPassed = factorsOfEach.every(arr => arr.indexOf(toTest) !== -1);

A Cleaner Solution

Using the Euclidean Algorithm to find the GCD/GCF of two numbers (assuming non-negative numbers) is a much more cleaner and optimized approach:

function GCF(a, b) {
    if (b === 0) return a;
    else         return GCF(b, a % b);
}

Then, just use a reduce statement to apply GCF to all of the numbers in the array:

function findGCFofList(list) {
    return list.reduce(GCF);
}

    
deleted 5 characters in body
Source Link

Formatting

Before addressing optimizations, let's address some formatting problems

while vs. for loops

It is preferred to use a for loop when you both:

  1. Have an incrementer
  2. The incrementer is not used outside of the loop
  3. The incrementer is either strictly increasing or strictly decreasing

This is the case with both of your while loops. They can be refactored to the following:

In listFactors():

for (var i = 2; i <= number; i++){
    if(isFactor(number, i)){
        factors.unshift(i);
    }
}

In GCF():

for (var count = 0; count < factorsOfFirst.length; count++){
    var toTest = factorsOfFirst[count];
    var passTest = factorsOfEach.every(arrayContains);
    if (passTest) {
        return toTest;
    }
}

Avoid explicit booleans when possible

Instead of:

if(num % fact === 0){
    return true;
}
else{
    return false;
}

it is preferable to write:

return num % fact === 0;

and the same for your other functions.

Avoid global variables

As you correctly pointed out, you should try not to use mutable global variables; it makes the code much harder to understand. To avoid this, you could make the arrayContains function merely an [arrow] function(https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functionsarrow function) and pass in toTest as well:

var testPassed = factorsOfEach.every(arr => arr.indexOf(toTest) !== -1);

A Cleaner Solution

Using the Euclidean Algorithm to find the GCD/GCF of two numbers (assuming non-negative numbers) is a much more cleaner and optimized approach:

function GCF(a, b) {
    if (b === 0) return a;
    if (a < b)  return GCF(b, a);
    else        return GCF(b, a % b);
}

Then, just use a reduce statement to apply GCF to all of the numbers in the array:

function findGCFofList(list) {
    return list.reduce(GCF);
}

    

Formatting

Before addressing optimizations, let's address some formatting problems

while vs. for loops

It is preferred to use a for loop when you both:

  1. Have an incrementer
  2. The incrementer is not used outside of the loop
  3. The incrementer is either strictly increasing or strictly decreasing

This is the case with both of your while loops. They can be refactored to the following:

In listFactors():

for (var i = 2; i <= number; i++){
    if(isFactor(number, i)){
        factors.unshift(i);
    }
}

In GCF():

for (var count = 0; count < factorsOfFirst.length; count++){
    var toTest = factorsOfFirst[count];
    var passTest = factorsOfEach.every(arrayContains);
    if (passTest) {
        return toTest;
    }
}

Avoid explicit booleans when possible

Instead of:

if(num % fact === 0){
    return true;
}
else{
    return false;
}

it is preferable to write:

return num % fact === 0;

and the same for your other functions.

Avoid global variables

As you correctly pointed out, you should try not to use mutable global variables; it makes the code much harder to understand. To avoid this, you could make the arrayContains function merely an [arrow] function(https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions) and pass in toTest as well:

var testPassed = factorsOfEach.every(arr => arr.indexOf(toTest) !== -1);

A Cleaner Solution

Using the Euclidean Algorithm to find the GCD/GCF of two numbers (assuming non-negative numbers) is a much cleaner and optimized approach:

function GCF(a, b) {
    if (b === 0) return a;
    if (a < b)  return GCF(b, a);
    else        return GCF(b, a % b);
}

Then, just use a reduce statement to apply GCF to all of the numbers in the array:

function findGCFofList(list) {
    return list.reduce(GCF);
}

    

Formatting

Before addressing optimizations, let's address some formatting problems

while vs. for loops

It is preferred to use a for loop when you:

  1. Have an incrementer
  2. The incrementer is not used outside of the loop
  3. The incrementer is either strictly increasing or strictly decreasing

This is the case with both of your while loops. They can be refactored to the following:

In listFactors():

for (var i = 2; i <= number; i++){
    if(isFactor(number, i)){
        factors.unshift(i);
    }
}

In GCF():

for (var count = 0; count < factorsOfFirst.length; count++){
    var toTest = factorsOfFirst[count];
    var passTest = factorsOfEach.every(arrayContains);
    if (passTest) {
        return toTest;
    }
}

Avoid explicit booleans when possible

Instead of:

if(num % fact === 0){
    return true;
}
else{
    return false;
}

it is preferable to write:

return num % fact === 0;

and the same for your other functions.

Avoid global variables

As you correctly pointed out, you should try not to use mutable global variables; it makes the code much harder to understand. To avoid this, you could make the arrayContains function merely an arrow function and pass in toTest as well:

var testPassed = factorsOfEach.every(arr => arr.indexOf(toTest) !== -1);

A Cleaner Solution

Using the Euclidean Algorithm to find the GCD/GCF of two numbers (assuming non-negative numbers) is a much more cleaner and optimized approach:

function GCF(a, b) {
    if (b === 0) return a;
    if (a < b)  return GCF(b, a);
    else        return GCF(b, a % b);
}

Then, just use a reduce statement to apply GCF to all of the numbers in the array:

function findGCFofList(list) {
    return list.reduce(GCF);
}

    
Source Link

Formatting

Before addressing optimizations, let's address some formatting problems

while vs. for loops

It is preferred to use a for loop when you both:

  1. Have an incrementer
  2. The incrementer is not used outside of the loop
  3. The incrementer is either strictly increasing or strictly decreasing

This is the case with both of your while loops. They can be refactored to the following:

In listFactors():

for (var i = 2; i <= number; i++){
    if(isFactor(number, i)){
        factors.unshift(i);
    }
}

In GCF():

for (var count = 0; count < factorsOfFirst.length; count++){
    var toTest = factorsOfFirst[count];
    var passTest = factorsOfEach.every(arrayContains);
    if (passTest) {
        return toTest;
    }
}

Avoid explicit booleans when possible

Instead of:

if(num % fact === 0){
    return true;
}
else{
    return false;
}

it is preferable to write:

return num % fact === 0;

and the same for your other functions.

Avoid global variables

As you correctly pointed out, you should try not to use mutable global variables; it makes the code much harder to understand. To avoid this, you could make the arrayContains function merely an [arrow] function(https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions) and pass in toTest as well:

var testPassed = factorsOfEach.every(arr => arr.indexOf(toTest) !== -1);

A Cleaner Solution

Using the Euclidean Algorithm to find the GCD/GCF of two numbers (assuming non-negative numbers) is a much cleaner and optimized approach:

function GCF(a, b) {
    if (b === 0) return a;
    if (a < b)  return GCF(b, a);
    else        return GCF(b, a % b);
}

Then, just use a reduce statement to apply GCF to all of the numbers in the array:

function findGCFofList(list) {
    return list.reduce(GCF);
}