Multithreaded array summation in Java - best practice, structure, acceptable use of Constant classes. There is no concurrency?

Question

The task is educational. The basic functionality is working. Here are some questions for my code. Please give me feedback.

Main:

1. Architecture & Design: Is the decomposition appropriate? Should SummatorService be split further or is utility class pattern justified here?

2. Thread Safety: Are there any race conditions I missed? The code doesn't use synchronized - is this correct for this implementation?

3. Performance: Are there obvious performance bottlenecks? Would using ExecutorService/ThreadPool be better than manual thread management?

4. Error Handling: What error scenarios should I handle? (NumberFormatException, array size edge cases, etc.)

Detail:

5. Code Style: Any violations of Java conventions or best practices you notice?

6. Memory Usage: Could the current approach cause memory issues with large arrays? Any optimization suggestions?

7. Testing: What unit tests would you write for this code? Which edge cases am I missing?

8. Scalability: How would you modify this for production use? What's missing for real-world application?

Alternatives:

9. Different Approaches: Would you implement this differently? (Fork/Join, parallel streams, etc.)

10. Industry Standards: How does this compare to how such problems are typically solved in enterprise Java?

11. Do I understand correctly that it is also possible to fill an array with random values in parallel streams, and this would be a good practice?

The task (as rendered text):

Excercise 02 - Real Multithreading
Allowed types: Object, Thread, Runnable
Try to use multithreading for its intended purpose: distribute computations

Let's assume there is an array of integer values. Your goal is to calculate the sum of array elements using several "summing" threads.
Each thread computes a certain section inside the array. The number of elements in each section is identical, except for the last one (its size can differ upward or downward).
The array shall be randomly populated each time. Array length and number of threads are passed as command-line arguments.
To make sure the program operates correctly, "we" should start by calculating the sum of array elements using a standard method.
Maximum number of array elements is 2,000,000. Maximum number of threads is no greater than the number of array elements. Maximum modulo value of each array element is 1,000. All data is guaranteed to be valid.

Example of program operation (each array element equals 1):

$ java Program --arraySize=13 --threadCount=3
Sum: 13
Tread 1: from 0 to 4 sum is 5
Tread 2: from 5 to 9 sum is 5
Tread 3: from 10 to 12 sum is 3
Sum by the threads: 13

Notes:

• In the above example, the size of the last summing-up section used by the third thread is less than others.

Here is the code:

package ex02;
import java.util.Arrays;
/**
 * Main entry point for multithreaded array summation program.
 * Accepts command line arguments for array size and thread count,
 * creates the array and runs summators in parallel.
 */
public class Program {
 /**
 * Application main method. Checks arguments, generates array, runs threads and
 * prints results.
 * 
 * @param args Command line arguments: --arraySize=N --threadsCount=N
 * @throws InterruptedException If thread interruption occurs during join
 */
 public static void main(String[] args) throws InterruptedException {
 if (SummatorService.isArgsChecked(args)) {
 int arraySize = SummatorService.getArraysize(
 args[0].substring("--arraySize=".length()));
 int threadsCount = SummatorService.getCountOfThreads(arraySize,
 args[1].substring("--threadsCount=".length()));
 int[] array = SummatorService.fillArrayRandomValues(
 arraySize);
 Summator[] summators = SummatorService.createArrayWithRunnable(
 threadsCount, array);
 System.out.println("Sum: " + Arrays.stream(array).sum());
 Thread[] threads = new Thread[threadsCount];
 for (int i = 0; i < threads.length; i++) {
 threads[i] = new Thread(summators[i]);
 threads[i].start();
 }
 for (Thread thread : threads) {
 thread.join();
 }
 System.out.println("Sum by threads: "
 + Arrays.stream(summators)
 .mapToInt(Summator::getResult)
 .sum());
 }
 }
}
package ex02;
/**
 * Runnable that sums a segment of an integer array.
 * Stores its own thread index, boundaries, and result.
 */
public class Summator implements Runnable {
 /** Reference to the source array to sum */
 private int[] arrayToSum;
 /** Start index (inclusive) of segment to sum */
 private int indexStart;
 /** End index (exclusive) of segment to sum */
 private int indexEnd;
 /** Index of thread for output purposes */
 private int threadIndex;
 /** Index of thread for output purposes */
 private int result = 0;
 /**
 * Summator constructor.
 * 
 * @param array Source array
 * @param start Start index (inclusive)
 * @param end End index (exclusive)
 * @param numberOfThread Thread index (for reporting)
 */
 public Summator(int[] array, int start, int end, int numberOfThread) {
 arrayToSum = array;
 indexStart = start;
 indexEnd = end;
 threadIndex = numberOfThread;
 }
 /**
 * Runs the summation for the assigned segment.
 * Prints sum and range for this thread.
 */
 @Override
 public void run() {
 for (int i = indexStart; i < indexEnd; i++) {
 result += arrayToSum[i];
 }
 System.out.printf("Thread %d: from %d to %d sum is %d%n",
 threadIndex,
 indexStart,
 indexEnd - 1,
 result);
 }
 /**
 * Gets the resulting sum calculated by this instance.
 * 
 * @return Resulting sum
 */
 public int getResult() {
 return result;
 }
}
package ex02;
import java.util.Random;
/**
 * Utility class for array generation, input parsing,
 * thread count calculation, and constructing Summator instances.
 */
public class SummatorService {
 /** Maximum allowed size of the array */
 private static final int MAX_ARRAY_SIZE = 2_000_000;
 /** Maximum modulus value for array elements */
 private static final int MAX_VALUE_FOR_RANDOM = 1_000;
 /**
 * Utility constructor. Prevents instantiation.
 */
 private SummatorService() {
 throw new UnsupportedOperationException("Utility class");
 }
 /**
 * Checks whether input arguments are valid and match the expected format.
 * 
 * @param args Command line arguments
 * @return true if valid, false otherwise
 */
 public static boolean isArgsChecked(String[] args) {
 boolean isValid = true;
 if (args.length != 2) {
 System.out.println("Usage of program: "
 + "java Program --arraySize=N --threadsCount=N");
 isValid = false;
 } else if (!args[0].startsWith("--arraySize=")) {
 System.out.println("First argument must be "
 + "--arraySize=N, now it is: " + args[0]);
 isValid = false;
 } else if (!args[1].startsWith("--threadsCount")) {
 System.out.println("Second argument must be "
 + "--threadsCount=N, now it is: " + args[1]);
 isValid = false;
 }
 return isValid;
 }
 /**
 * Parses array size string and applies boundaries if out of allowed range.
 * 
 * @param arraySizeString String representing the array size
 * @return final array size in bounds
 */
 public static int getArraysize(String arraySizeString) {
 int arraySize = Integer.parseInt(arraySizeString);
 if (arraySize < 1) {
 arraySize = 1;
 } else if (arraySize > MAX_ARRAY_SIZE) {
 arraySize = MAX_ARRAY_SIZE;
 }
 return arraySize;
 }
 /**
 * Parses thread count string, clamps to array size if exceeding it.
 * 
 * @param arraySize Size of the array
 * @param threadCountString String representing thread count
 * @return valid thread count
 */
 public static int getCountOfThreads(int arraySize, String threadCountString) {
 int threadsCount = Integer.parseInt(threadCountString);
 if (threadsCount > arraySize) {
 threadsCount = arraySize;
 }
 return threadsCount;
 }
 /**
 * Generates an array of random integers in range [0, MAX_VALUE_FOR_RANDOM)
 * 
 * @param capacity array size
 * @return new int array
 */
 public static int[] fillArrayRandomValues(int capacity) {
 int[] array = new int[capacity];
 Random random = new Random();
 for (int i = 0; i < array.length; i++) {
 array[i] = random.nextInt(MAX_VALUE_FOR_RANDOM);
 }
 return array;
 }
 /**
 * Creates an array of Summator runnables, each responsible for its own chunk of
 * the array.
 * 
 * @param numberOfThreads Number of summator threads
 * @param srcArray Source array for summation
 * @return array of Summator instances
 */
 public static Summator[] createArrayWithRunnable(int numberOfThreads,
 int[] srcArray) {
 Summator[] array = new Summator[numberOfThreads];
 int subArrayCapacity = srcArray.length / numberOfThreads;
 int index = 0;
 for (int i = 0; i < numberOfThreads - 1; i++) {
 array[i] = new Summator(srcArray, index, index + subArrayCapacity, i + 1);
 index += subArrayCapacity;
 }
 array[numberOfThreads - 1] = new Summator(srcArray, index,
 srcArray.length, numberOfThreads);
 return array;
 }
}

• greybeard: Please don't use images of rendered text: Paste the text into a block quote - proper attribution required.
• greybeard: I don't understand "modulo value of each array element". I don't understand the title's "There is no concurrency?"
• Peter Cordes: @greybeard: I solved the first mystery: searching on "modulus" found the comment on the definition of MAX_VALUE_FOR_RANDOM. So apparently they're assuming that random.nextInt(MAX_VALUE_FOR_RANDOM) is implemented internally as random % maxval, instead of a method which avoids introducing bias for ranges which aren't powers of 2. (And this assumption is present even in the assignment. Yikes.)
• bonnyped: @greybeard modified
• greybeard: ("modified" - ? I see no modification. My priorities: 0) Who created the text in the image/block quote? (omitting this is not an option) 1) What does "There is no concurrency?" mean? 2) What does "(Maximum) modulo value of each array element" mean?)

Answer 1

SRP

There is a school of thought out there that writing an utility class is indicative of a design smell. While I don't always agree, particularely with small pet projects like this, SummatorService does stretch the limits here. We have:

• elements of a Command Line Parser (isArgsChecked(...), Validation)
• Random Generation (fillArrayRandomValues())
• Scheduler fragments (createArrayWithRunnable(...))

consider seperating those.

Overall Design

• Consider changing the Program so the order of arguments doesn't matter. Ordered arguments are mainly seen in larger programs that have submodules (the java command is a prime example, everything before -jar someFile.jar or ex02.Program is used by the VM, the rest is passed to your app).
• Its contrary to the task description, but when the solution requires a full CPU-bound calculation like here, there is little use in using more threads than the box this runs on has cores (You're just contending with yourself for CPU access). Consider hard-capping the thread count at Runtime.getRuntime().availableProcessors(). (Not to mention your kernel might have a thing or two to say about someone spawning 2m threads, too)
• Matching the number of runnables (or futures/callables, if you'd used a ThreadPool) to the number of threads is almost always counter-productive. In this case it probbably doesn't matter, since threads summing over largely identical-length array slices don't have enough wiggle room (or work to run for meaningfull amounts of time), but in practice, and particularely with thread pools, you'd split the whole task into more slices to minimize effects from some threads finishing their (1st) task sooner than others.
• Summator.getResult() should probbably throw an IllegalStateException if the results aren't available yet.
• the fields in Summator can get a few extra modifiers: result can become volatile (You're getting away without it here since nothing retrieves the value from another thread until the run method has completed, but it makes me nearvous if a single refactoring elsewhere can introduce a thread concurrency bug here) and the other variables aren't modified and so can become final.
• SummatorService.isArgsChecked should probbably reject bad input (e.g. --arraySize=foobar) upfront, rather than letting later code throw a NFE. Alsp, print error messages to System.err.

Naming

• I'd call SummatorService.isArgsChecked SummatorService.validateArguments or somesuch instead, to reduce the element of suprise when you find out that it actually checks.
• getArraysize -> getArraySize
• I'd favor getNewRandomlyFilledArray(int size) over fillArrayRandomValues(capacity), to reduce the element of surprise that you don't get to provide your own array.

Testing

• consider making the Random in SummatorService a field. This allows tests to inject a Random with a given seed, making tests easier because the results don't depend upon what random rolls come out.
• SummatorService.isArgsChecked: 1 assertEquals on the return value per branch. getArraysize and getCountOfThreads ditto.
• fillArrayRandomValues(capacity): 1 assertEquals on a return value, with a suitably large capacity. And the edge case capacity <=0
• At least one full run with a set seed for the randomness. This would be easier if main would delegate the main bulkwork to a method sumArray(int arraySize, int threadCount) which returns the sum; or somesuch method, allowing us to get the counts without mocking System.out

Misc. Points

• Consider turning the if in main into a guard clause, to avoid code croaching to the right screen if you have a few of those, especially with 8-space idents. Also, use System.exit here so we get a proper exit code.

   if (!SummatorService.isArgsChecked(args)) {
   System.exit(1);
   return;
   }
   //...
  

• Array filling can be made easier with streams:

   random.ints(capacity, 0 /* origin*/ , MAX_VALUE_FOR_RANDOM).parallel().toArray();
  

• In practice you'd probbably execute this on a threadpool, and quite possibly ForkJoinPool.commonPool(). Also, the summing might need to use a long, or maybe even a BigInteger, to avoid nummerical overflow.

• I don't know where your instructor expected to use syncronized here. Syncronizing on the array instance kind of defeats the use of using threads, and syncronizing on a lock while modifying a shared result variable does too, not to mention the contention as threads continually get in each others way.

• Summator.result has a bad (copy-pasted) javadoc comment.

• Peter Cordes: BigDecimal instead of BigInteger? Why decimal floating-point for an integer sum? BigDecimal uses an arbitrary-precision unscaled part, so it's like BigInteger plus a 32-bit scale factor. (docs.oracle.com/javase/8/docs/api/index.html?java/math/…). Overhead won't matter much if you only accumulate long into BigDecimal (or BigInteger) after summing chunks of up to 2^32 integers, i.e. once per thread work unit, so that upper bound is plenty high for any reasonable omp_schedule(dynamic) type of strategy like you're recommending (non-huge chunks).
• Jannik S.: @PeterCordes BigDecimal was an oversight, thank you for catching it.

Answer 2

Clean structure and the demo works. A few changes will make it safer and more 'Java in 2025'.

1. Use ExecutorService with Callable<Integer> instead of raw Thread + shared state.
   Thread + Runnable forces you to store partial sums in a mutable field and then read them later. With an executor you return values, get clear error handling, and you can size the pool to CPU cores.

 var pool = Executors.newFixedThreadPool(Math.min(threadsCount, Runtime.getRuntime().availableProcessors()));
 try {
 List<Callable<Integer>> tasks = new ArrayList<>();
 int base = array.length / threadsCount;
 int rem = array.length % threadsCount;
 int start = 0;
 for (int i = 0; i < threadsCount; i++) {
 int len = base + (i < rem ? 1 : 0); // fair split
 int s = start, e = s + len;
 tasks.add(() -> {
 int sum = 0;
 for (int k = s; k < e; k++) sum += array[k];
 return sum;
 });
 start = e;
 }
 int total = 0;
 for (Future<Integer> f : pool.invokeAll(tasks)) total += f.get();
 System.out.println("Sum by threads: " + total);
 } finally {
 pool.shutdown();
 }

Why this is better: no shared mutable fields, exceptions surface via Future.get, and you control parallelism.

2. Fix arg parsing edge cases

• --threadsCount should be validated with the equals sign like the first arg: check startsWith("--threadsCount=").
• Clamp thread count to [1, arraySize]. Right now 0 will cause a divide-by-zero when you compute the chunk size.

 public static int getCountOfThreads(int arraySize, String s) {
 int n = Integer.parseInt(s);
 if (n < 1) n = 1;
 if (n > arraySize) n = arraySize;
 return n;
 }

3. Make workers quiet and keep I/O at the edges
   Printing from each worker adds contention and makes timing noisy. Have workers compute sums only. Print once in main after aggregation.

Nice to have

• Consider Arrays.parallelSetAll(array, i -> ThreadLocalRandom.current().nextInt(MAX_VALUE_FOR_RANDOM)) for parallel fill when the exercise calls for it.
• Use long for totals if you ever raise the bounds. It prevents accidental overflow with very large arrays.

Answer 3

I like the (doc-) comments very much.

Inconsistently, class Programis indented more than 4 spaces per level.