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

Finding divisors of a number

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

Problem

This code finds all the divisors of a given number. Can it be shortened?

import java.util.Scanner;

public class PrimeNum2{
    public static void main(String args[]){
        Scanner x=new Scanner(System.in);
        System.out.print("Enter the number :  ");
        long y=x.nextInt(),i;
        System.out.print("Divisors of "+y+" = 1 , ");

        for( i=2;i<y;i++){
            long z=y%i;
            if(z!=0)continue;
                System.out.print(i+" , ");

        }System.out.println(y);
    }
}

Solution

This code could do with some editing...

First of all is the spacing. It is absolutely horrible (we will fix that after the edits).

Also, the naming is horrible. Scanner x could be scanner and y could be num. As for z, it is completely unnecessary:

for (i = 2; i < y; i++) {
    long z = y % i;
    if (z != 0)
        continue;
    System.out.print(i + " , ");
}


Becomes:

for (i = 2; i < y; i++) {
    if (y % i != 0)
        continue;
    System.out.print(i + " , ");
}


The program can do without the continue statement:

for (i = 2; i < y; i++) {
    if (y % i == 0)
        System.out.print(i + " , ");
}


It's also a good idea to put braces around statements in an if statement, even when there is only one:

for (i = 2; i < y; i++) {
    if (y % i == 0) {
        System.out.print(i + " , ");
    }
}


You are also wasting time going through for loops doing nothing. After all, num's largest factor before itself possible is num / 2, which makes it more efficient doing it like this:

for (i = 2; i <= num / 2; i++) {
    if (num % i == 0) {
        System.out.print(i + " , ");
    }
}


I also noticed:

public static void main(String args[])


It is better to put [] at the type (String):

public static void main(String[] args)


But the main problem is that you have a memory leak. It could be solved by closing the Scanner:

scanner.close();


Final code:

Your final code will look like this:

public class PrimeNum2 {
    public static void main(String args[]) {
        Scanner scanner = new Scanner(System.in);
        System.out.print("Enter the number :  ");
        long num = scanner.nextInt(), i;
        System.out.print("Divisors of " + num + " = 1 , ");
        for (i = 2; i <= num / 2; i++) {
            if (num % i == 0) {
                System.out.print(i + " , ");
            }
        }
        System.out.println(num);
        scanner.close();
    }
}

Code Snippets

for (i = 2; i < y; i++) {
    long z = y % i;
    if (z != 0)
        continue;
    System.out.print(i + " , ");
}
for (i = 2; i < y; i++) {
    if (y % i != 0)
        continue;
    System.out.print(i + " , ");
}
for (i = 2; i < y; i++) {
    if (y % i == 0)
        System.out.print(i + " , ");
}
for (i = 2; i < y; i++) {
    if (y % i == 0) {
        System.out.print(i + " , ");
    }
}
for (i = 2; i <= num / 2; i++) {
    if (num % i == 0) {
        System.out.print(i + " , ");
    }
}

Context

StackExchange Code Review Q#36837, answer score: 20

Revisions (0)

No revisions yet.