Prime factorization of a natural number greater than 1

I want to write a program in Java that return a prime factorization of any natural number greater than one.

After spending some time coding and debugging, the program runs fine. Since I am new to Java and programming in general, I don't know how bad my code style and class/method design are. Hence, I am here asking for some tips/advice on what I did wrong and what I could do to improve.

```
public class FTA_Prime_Factorization {

/**
* A method to ask for a integer input from user.
*
* @return num: int
*/
protected static int user_input() {
int num;
Scanner inp;
while (true) {
inp = new Scanner(System.in);
System.out.print("Please enter a greater-than-1 positive integer: #");
if (inp.hasNextInt() && (num = inp.nextInt())>1)
break;
System.out.println("Please enter a integer greater than 1!!!");

}
return num;
}

/**
* To check whether the parameter is a prime number.
*
* @param x: int
* @return counts == 2: boolean
*/
private static boolean checkPrime (int x) {
int counts = 0;
for (int i = x; i > 0; i--) {
if (x % i == 0) {
counts++;
}
}
return counts == 2;
}

/**
* To create an Arraylist of prime number that is less than 10000.
*
* @return primeList: Arraylist
*/
private static ArrayList primes() {
ArrayList primeList = new ArrayList<>();
for (int x = 1; x
*/
private static ArrayList array(int x) {
ArrayList primeFactors = new ArrayList<>();
ArrayList listOfPrime = primes();
for (int i : listOfPrime) {
for (int j = x; j % i== 0; j /= i)
primeFactors.add(i);
}
return primeFactors;
}

public static void main(String [] args) {
ArrayList temp = array(user_input());
HashMap counter = n

Method names

You should try to make the name of your methods are descriptive as possible, without being too long:

• user_input should first be userInput according to Java naming conventions. But it is difficult to tell what the method does by just looking at its name: it surely involves the user input but we don't know how and why. In this case, the purpose of the method is to retrieve a number to factor. A possible name would be getNumberToFactor or askNumberToFactor, although I would prefer the first variant because it describes what the method does and does not care of how it does it: in the future, you might expand this code and read the number from a file for example; the method's purpose would still be the same, and its name wouldn't change.

• checkPrime could be renamed isPrime: generally, boolean-returning methods have a name starting with is (or has).

• primes is fine: it returns a list of prime numbers and it is well described that this name. (Perhaps getPrimes() but this is a matter of opinion)

• array is misleading. One would assume that it makes an array of something when it fact, this is the prime method making the factorization. It would be better to name it primeFactors and make it clear that this method makes the prime factorization of the given parameter.

Programming to an interface

It is a good idea to program to an interface. This means, trying to avoid the implementing classes and work only on the interfaces. In this case, you are passing ArrayList when you should be passing List:

//  v--v
private static List primes() {
    List primeList = new ArrayList<>();
 // ^--^
    for (int x = 1; x < 10000; x++){
        if (checkPrime(x))
            primeList.add(x);
    }
    return primeList;
}

Map counter = new HashMap<>();
//  ^-^

Separated logic

You have reasonably well separated the code into methods, but the main array is missing its main ingredient. This method is the central-point for making the prime factorization. Therefore, it is that method that should direcly return a Map of the prime factors with their exponent.

Currently, this code is separated in two unrelated blocks:

private static ArrayList array(int x) {
    // return list of primes dividing x
}

public static void main(String [] args) {
    ArrayList temp = array(user_input());
    HashMap counter = new HashMap<>();
    // process that list and turn it into a Map
}

Note that separating this code actually hides you from an optimization you can make: you don't need to have a list of all primes factors with duplicate values. You can just construct the Map directly, without having that temporary list:

private static Map primeFactors(int x) {
    Map primeFactors = new HashMap<>();
    ArrayList listOfPrime = primes();
    for (int i : listOfPrime) {
        for (int j = x; j % i == 0; j /= i) {
            if (primeFactors.get(i) != null) {
                primeFactors.put(i, primeFactors.get(i) + 1);
            } else {
                primeFactors.put(i, 1);
            }
        }
    }
    return primeFactors;
}

If you're using Java 8, you can make this even simpler by replacing the updating of the map with counter.merge(i, 1, Integer::sum);:

private static Map array(int x) {
    Map counter = new HashMap<>();
    ArrayList listOfPrime = primes();
    for (int i : listOfPrime) {
        for (int j = x; j % i == 0; j /= i) {
            counter.merge(i, 1, Integer::sum);
        }
    }
    return counter;
}

Avoid concatenating Strings with +=

In the following loop

for (int key : counter.keySet()) {
    primeFactors += counter.get(key) == 1 ? "(prime)" + key + " x "
            : "(prime)" + key + "^" + counter.get(key) + " x ";
}

you are concatenating the primeFactors variables with +=. This is not a good practice because it creates a lot of Strings in memory. It would be better to use a StringBuilder for this task and append each part with append(...).

Also, I don't see the need to print (prime) each time: you know it's a prime already since we're doing the prime factorization.

If you're using Java 8, you can actually use StringJoiner instead: this will also enable you to get rid of the

primeFactors.substring(0, primeFactors.length() - 3)

at the end. This class is designed to join Strings together with a separator:

StringJoiner sj = new StringJoiner(" ");
for (int key : counter.keySet()) {
    sj.add(counter.get(key) == 1 ? key + " x" : key + "^" + counter.get(key) + " x");
}
System.out.println(sj.toString());

StackExchange Code Review Q#121482, answer score: 10