CODEWITHSUNDEEP
javascriptdebuggingcomparison
Computernerd
Computernerd

Reputation: 7766

Find highest common factor between two numbers

I am trying to determine the highest common factor between two numbers. This is my code

function Division(num1,num2) { 

  var sum = 1

  //contain all divisor which can divide num without remainder
  var num1_divisor = []
  var num2_divisor = []

  //if num1%divisor == 0 , store in num1 divisor
  for (var i = 1 ; i < num1 ; i++)
  {
     if ( num1 % i == 0 )
     {
        num1_divisor.push(i)
     }
  }

  console.log(num1_divisor)

  //if num2%divisor == 0 , store in num1 divisor
   for (var i = 1 ; i < num1 ; i++)
  {
     if ( num2 % i == 0 )
     {
        num2_divisor.push(i)
     }
  }

  console.log(num2_divisor)

  //if num1_divisor is contained in num2 divisor
  //mulitply them by sum
  for ( var i = 0 ; i < num1.length ; i++)
  {
        var num1_value = num1_divisor[i]

       for ( var j = 0 ; j < num2.length ; j++)
       {
         var num2_value = num2_divisor[j]

          if (num1_value == num2_value )
          {
            sum = sum * num2_value

          }
       }
  }

         return sum
}

The logic of the program is as follows : For num1 and num2, it will store all divisible values in num1_divisor and num2_divisor respectively. I will then multiply all common values between num1_divisor and num2_divisor to find the highest common factor between both numbers.

I have checked the program and there seems to be an error with the comparing portion

 if (num1_value == num2_value )
              {
                sum = sum * num2_value

              }

For reasons unknown to me, num1_value does not equal num2_value though both numbers are the same.

A sample case would be Division(10,12), it returns 1 though it should return 2

I would appreciate any help

Thanks

Upvotes: 0

Views: 1157

Answers (3)

Mahesh Kapoor
Mahesh Kapoor

Reputation: 254

I found the issue in your code, the problem is not where you are suspecting but in the two for loops, where you are finding the length of the array of divisor. Instead of using "num1.length" you need to use "num1_divisor.length". similarly instead of "num2.length", you need to us "num2_divisor.length"

Corrected version is available below:

function Division(num1,num2) { 

  var sum = 1

  //contain all divisor which can divide num without remainder
  var num1_divisor = []
  var num2_divisor = []

  //if num1%divisor == 0 , store in num1 divisor
  for (var i = 1 ; i < num1 ; i++)
  {
     if ( num1 % i == 0 )
     {
        num1_divisor.push(i)
     }
  }

 //alert(num1_divisor)

  //if num2%divisor == 0 , store in num1 divisor
   for (var i = 1 ; i < num1 ; i++)
  {
     if ( num2 % i == 0 )
     {
        num2_divisor.push(i)
     }
  }

 //alert(num2_divisor)

  //if num1_divisor is contained in num2 divisor
  //mulitply them by sum
  //alert(num2_divisor.length);
  for ( var i = 0 ; i < num1_divisor.length ; i++)
  {
     // alert(i);
        var num1_value = num1_divisor[i]

       for ( var j = 0 ; j < num2_divisor.length ; j++)
       {
         var num2_value = num2_divisor[j]
//alert( num1_value);
//alert( num2_value);           
          if (num1_value == num2_value )
          {
              //alert(sum);
            sum = sum * num2_value

          }
       }
  }

         return sum
}

You may run your code here

Upvotes: 1

Joseph
Joseph

Reputation: 119837

Here's a simpler version of looking for the greatest common factor, which should 

// Generate an array from 0 to limit
function range(limit){
  return Array.apply(null, Array(limit)).map(function (_, i) {return i;});
}

function greatestCommonFactor(number1, number2){
  // Figure out who's larger and smaller
  var largerNumber = Math.max(number1, number2);
  var smallerNumber = Math.min(number1, number2);

  // Use a range to avoid using counters and increments
  var numbersUpToLargest = range(largerNumber + 1).slice(1);

  // Find the numbers that divide the larger number perfectly
  var largeNumberFactors = numbersUpToLargest.filter(function(number){
    return !(largerNumber % number);
  });

  // With the larger number's factors, find the ones that divide the smaller number perfectly
  var commonFactors = largeNumberFactors.filter(function(factor){
    return !(smallerNumber % factor);
  });

  // Since we ordered from lowest to highest, reverse the array and get the first.
  return commonFactors.reverse()[0];
}

alert(greatestCommonFactor(10, 12));

Upvotes: 1

keune
keune

Reputation: 5795

There is no length of a number, you probably wanted to check num1_divisor.length instead of num1.length and num2_divisor.length instead of num2.length

Also, you need to make sure the loops that you use to find divisors include the number itself, because a number is a divisor of itself.

for (var i = 1 ; i <= num1 ; i++)

and

for (var i = 1 ; i <= num2 ; i++)

After these changes, the result seems to be correct.

Upvotes: 1

Related Questions