Calculating the Factorial of a number I am looking for feedback for the implementation for calculating factorial of a number. I have tried to do with a loop, recursion and in a mutlithreaded way.

package org.spituk.study.programs;

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.*;

/**
 * Class for calculating factorial of a number.
 *
 * @author Karan Khanna
 * @version 1.0
 * @since 12/8/2018
 */
public class Factorial {

    private final static int JOB_SIZE = 10;

    private final static int AVAILABLE_PROCESSORS = Runtime.getRuntime().availableProcessors();

    /**
     * Calculates the factorial of the number with loop.
     *
     * @param number whose factorial is to be calculated.
     * @return calculated factorial of the number
     */
    public long getFactorialWithLoop(final int number) {
        long factorial = 1;
        for (int index = 2; index &lt;= number; index++) {
            factorial = factorial * index;
        }
        return factorial;
    }


    /**
     * Calculates the factorial of the number with recursion.
     *
     * @param number whose factorial is to be calculated.
     * @return calculated factorial of the number
     */
    public long getFactorialWithRecursion(final int number) {
        return factorialRecursion(number);
    }


    /**
     * Calculates the factorial of the number with multiple threads.
     *
     * @param number whose factorial is to be calculated.
     * @return calculated factorial of the number
     */
    public long getFactorialMultiThreaded(final int number) throws Exception {
        ExecutorService executor = Executors.newFixedThreadPool(AVAILABLE_PROCESSORS);
        List&lt;PartialFactorialJob&gt; partialFactorialJobs = new ArrayList&lt;PartialFactorialJob&gt;();

        int startIndex = 1;
        int pivot = JOB_SIZE;

        while(pivot &lt;= number) {
            if(pivot + JOB_SIZE &gt; number) {
                pivot = number;
            }
            partialFactorialJobs.add(new PartialFactorialJob(startIndex, pivot));
            startIndex = pivot + 1;
            pivot = pivot + JOB_SIZE;
        }

        try {
            long factorial = 1;
            final List&lt;Future&lt;Long&gt;&gt; futureList = executor.invokeAll(partialFactorialJobs);
            for (Future&lt;Long&gt; future : futureList) {
                factorial = factorial * future.get();
            }
            executor.shutdown();
            return factorial;
        } catch (InterruptedException | ExecutionException e) {
            throw new Exception("Factorial couldn't be calculated.");
        }
    }


    private long factorialRecursion(final int number) {
        if (number &lt;= 0) {
            return 1;
        }
        return number * factorialRecursion(number - 1);
    }

    private class PartialFactorialJob implements Callable&lt;Long&gt; {

        private final int startIndex;
        private final int endIndex;

        PartialFactorialJob(final int startIndex, final int endIndex) {
            this.startIndex = startIndex;
            this.endIndex = endIndex;
        }

        public Long call() {
            long partialFactorial = 1;
            int temp = startIndex;
            while (temp &lt;= endIndex) {
                partialFactorial = partialFactorial * temp;
                temp++;
            }
            return partialFactorial;
        }
    }
}
A few comments:



import java.util.concurrent.*;


Never do wildcard imports unless you're doing a one-time, throw-away program. Maybe, in Java 20 or so, they introduce a class java.util.concurrent.Factorial (contrived example), and then you have a name collision with one of your classes. Better stick to individual imports (or have your IDE organize the imports).



Thumbs up for attaching javadoc to all public elements.



public long getFactorialWithRecursion(final int number) {
    return factorialRecursion(number);
}


I'd skip the indirection from getFactorialWithRecursion() calling factorialRecursion() and write

public long getFactorialWithRecursion(final int number) {
    if (number &lt;= 0) {
        return 1;
    }
    return number * getFactorialWithRecursion(number - 1);
}




Other SO members already commented that computing a factorial with multiple threads might not be a useful application, but I understand that this is for exerecise purposes.

The while loop in getFactorialMultiThreaded makes it hard to read and understand the code, with the two loop variables startIndex and pivot having their operations spread over many lines of code. I'd write something like

for (int startIndex = 1; startIndex &lt;= number; startIndex += JOB_SIZE) {
    int endIndex = Math.min(startIndex + JOB_SIZE - 1, number);
    partialFactorialJobs.add(new PartialFactorialJob(startIndex, endIndex));
}


Similar thing applies to the while loop in your call method, I'd prefer a for loop instead.



As your PartialFactorialJob class doesn't access anything from the enclosing Factorial instance, you should make it static (otherwise it carries around a useless internal reference to its enclosing instance, typically visible in your debugger as this$1 or similar).