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

Greatest Common Factor

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

Problem

I wrote this program to determine the Greatest Common Factor of any 2 given numbers, and would like to know of any improvements that can be made. I have tested it using a loop and 2 randomly generated numbers and it seems to work perfectly:

import java.util.ArrayList;
import java.util.Scanner;

public class GreatestCommonFactor
{
    public static void main(String[] args)
    {
        System.out.println("Please enter 2 numbers:");
        Scanner in = new Scanner(System.in);

        int a = Integer.parseInt(in.nextLine());
        int b = Integer.parseInt(in.nextLine());
        int greatestCommonFactor = gcf(a,b);

        System.out.println(greatestCommonFactor);
        in.close();
    }

    public static int gcf(int a, int b)
    {
        int result = 0;
        ArrayList factorsA = factorise(a);
        ArrayList factorsB = factorise(b);

        for (int i = 0; i  factorise(int num)
    {
        ArrayList factors = new ArrayList();
        int bounds = num/2;
        for (int i = 1; i <= bounds; i++)
            if (num % i == 0)
                factors.add(i);
        factors.add(num);
        return factors;
    }
}

Solution

Good job on closing your Scanner! I see a lot of people forget/miss that.

An alternative, just for your information, is using try with resources so you don't have to worry about closing/remembering to close it. As an aside, in general, you want to use more meaningful names than just 'a' and 'b' in your code (Though I'm totally going to be a hypocrite in a second, names are hard).

I would consider using Scanner's nextInt() method, you're using integers, using nextLine() isn't intuitive. As it is, despite your request for two numbers(should specify integers), if a user actually tries to input two space delimited numbers it'll throw an error.

Here's what the start of your code would look like with these suggestions:

int first;
        int second;
        System.out.print("Please enter 2 integers: ");

        try(Scanner in = new Scanner(System.in)) {
            first = in.nextInt();
            second = in.nextInt();
        }


As for the actual logic of your code, You can use the Euclidean algorithm

// Greatest Common Denominator
public int computeGCD(int a, int b) {
    return b == 0 ? a : computeGCD(b, a % b);
}

Code Snippets

int first;
        int second;
        System.out.print("Please enter 2 integers: ");

        try(Scanner in = new Scanner(System.in)) {
            first = in.nextInt();
            second = in.nextInt();
        }
// Greatest Common Denominator
public int computeGCD(int a, int b) {
    return b == 0 ? a : computeGCD(b, a % b);
}

Context

StackExchange Code Review Q#87078, answer score: 14

Revisions (0)

No revisions yet.