HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Determining common divisors of two numbers

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
determiningnumberstwodivisorscommon

Problem

Is there a better algorithm for finding the common divisors of two numbers? Can this code be shortened?

import java.util.*;

public class Testing1 {
public static void main (String args[]){
    Scanner x=new Scanner(System.in);
    System.out.print("Enter the 1st number(larger number) :  ");
    int y=x.nextInt();
    System.out.print("Enter the 2nd number : ");
    int z=x.nextInt();

    int ar1[]=new int[y];
    int ar2[]=new int[z];

    for( int i=1;i<y;i++){ //puts the divisors of the larger number to an array ar1[]
         int l=y%i;
         if(l==0)
           ar1[i]=i;}

    for( int i=1;i<z;i++){//puts the divisors of the other number to an array ar2[]
        int l=z%i;
        if(l==0)
          ar2[i]=i;}System.out.print("Common divisors of "+y+", "+z+" = ");

    for(int i=1;i<=ar2.length-1;i++){ //printing the common elements of both arrays.
     if((ar1[i]==ar2[i])&&ar2[i]!=0)
        System.out.print(i+" ");
}}}

Solution

There are few improvements you can do in your code:

-
First choose better variable names. I would name the Scanner type variable as scanner rather than x. Really, x, y, z are the worst variable name, which doesn't give any idea as to what they denote.

-
Secondly, I won't force user to pass larger number first, and then smaller number. I would take the burden of finding that out myself.

-
There is no need of storing all the divisors of both the numbers in two different arrays. Suppose you have to find common divisor of 2 and 2132340. Will you store all the divisors of 2132340, or simply check that the common divisor cannot be greater than the divisor of 2, and use that fact?

-
Another fact is, all the divisors of num1 will be less than or equal to num1 / 2. Now, if num1

-
Of course, you can move the code that finds the common divisor in a different method, and store them in a
List`.

-
Lastly, it might be possible that the smaller number itself is a divisor of larger number. So, you need to store that too.

Here's the modified code which looks a bit cleaner:

public static void main (String args[]){
    Scanner scanner = new Scanner(System.in);

    System.out.print("Enter the 1st number :  ");
    int num1 = scanner.nextInt();
    System.out.print("Enter the 2nd number : ");
    int num2 = scanner.nextInt();

    List commonDivisor = getCommonDivisor(num1, num2);
    System.out.println(commonDivisor);
}

public static List getCommonDivisor(int num1, int num2) {

    List list = new ArrayList();

    int min = minimum(num1, num2);

    for(int i = 1; i <= min / 2; i++) { 
        if (num1 % i == 0 && num2 % i == 0) {
            list.add(i);
        }
    }

    if (num1 % min == 0 && num2 % min == 0) {
        list.add(min);
    }

    return list;
}

public static int minimum(int num1, int num2) {
    return num1 <= num2 ? num1 : num2;
}

Code Snippets

public static void main (String args[]){
    Scanner scanner = new Scanner(System.in);

    System.out.print("Enter the 1st number :  ");
    int num1 = scanner.nextInt();
    System.out.print("Enter the 2nd number : ");
    int num2 = scanner.nextInt();

    List<Integer> commonDivisor = getCommonDivisor(num1, num2);
    System.out.println(commonDivisor);
}

public static List<Integer> getCommonDivisor(int num1, int num2) {

    List<Integer> list = new ArrayList<Integer>();

    int min = minimum(num1, num2);

    for(int i = 1; i <= min / 2; i++) { 
        if (num1 % i == 0 && num2 % i == 0) {
            list.add(i);
        }
    }

    if (num1 % min == 0 && num2 % min == 0) {
        list.add(min);
    }

    return list;
}

public static int minimum(int num1, int num2) {
    return num1 <= num2 ? num1 : num2;
}

Context

StackExchange Code Review Q#36890, answer score: 7

Revisions (0)

No revisions yet.