refactor(control): split MainController state/session concerns by 66-m · Pull Request #37 · 66-m/sorting-visualizer

66-m · 2026-03-12T10:34:19Z

Extract controller constants and move runtime state/session orchestration into dedicated manager classes. Improve threading safety and error handling while keeping compatibility methods used by Settings and existing integrations.

Call updateGradient() to populate color array before passing to Bars and Circle visualizations. Fixes ArrayIndexOutOfBoundsException in CI test runs.

Copilot

Pull request overview

Refactors MainController by extracting configuration constants and moving mutable runtime state plus sorting-session orchestration into dedicated manager classes, aiming to improve structure and thread coordination while keeping legacy static entry points used by Settings and existing integrations.

Changes:

Added MainControllerConfig to centralize controller constants (window sizing, timing, rendering constants).
Introduced SortingStateManager to hold cross-thread mutable flags/state using atomics/volatile.
Introduced SortingSessionManager to run algorithms in a background thread and collect measurements, and updated MainController to delegate state/session behavior to these managers.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.

File	Description
src/main/java/io/github/compilerstuck/Control/SortingStateManager.java	New atomic/volatile-backed state holder for run lifecycle and UI flags.
src/main/java/io/github/compilerstuck/Control/SortingSessionManager.java	New background-thread session runner + measurement/timestamp collection.
src/main/java/io/github/compilerstuck/Control/MainControllerConfig.java	New constants class to replace magic numbers in controller logic/rendering.
src/main/java/io/github/compilerstuck/Control/MainController.java	Refactored to use the new config/state/session managers and preserve static compatibility methods.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-12T10:40:10Z

+ * the Settings UI and algorithm infrastructure. Access to these is coordinated
+ * through synchronized accessors.


Class Javadoc says access to compatibility statics is "coordinated through synchronized accessors", but the accessors below are not synchronized. Either add synchronization where needed, or adjust the documentation to avoid implying thread-safety guarantees that aren't present.

Suggested change

* the Settings UI and algorithm infrastructure. Access to these is coordinated

* through synchronized accessors.

* the Settings UI and algorithm infrastructure. These fields are retained for

* legacy integration and do not provide any additional thread-safety guarantees.

Copilot · 2026-03-12T10:40:10Z

    }

+    /**
+     * Directfieldaccess to array controller.


Typo in Javadoc: "Directfieldaccess" should be "Direct field access".

Suggested change

* Directfieldaccess to array controller.

* Direct field access to array controller.

Copilot · 2026-03-12T10:40:10Z

+    public void startSortingSession(List<SortingAlgorithm> algorithms) {
+        if (algorithms == null || algorithms.isEmpty()) {
+            LOGGER.log(Level.WARNING, "Attempted to start sorting session with empty algorithm list");
+            return;
+        }
+
+        // Clear previous results
+        clearMeasurements();
+
+        executionThread = new Thread(() -> executeSortingAlgorithms(algorithms));
+        executionThread.setName("SortingThread");
+        executionThread.start();
+    }


This new session orchestration logic is non-trivial (threading, cancellation, results/restart state transitions) but there are no corresponding unit tests. There are already JUnit tests under src/test/java/io/github/compilerstuck/Control/, so it would be good to add tests covering at least: (1) cancel/run=false stops the session cleanly without getting stuck in results state, and (2) running with audio unavailable does not throw (HeadlessSound / null-safety).

Copilot · 2026-03-12T10:40:11Z

+        // Initialize sound system
        try {
            sound = new MidiSys(arrayController);
        } catch (MidiUnavailableException e) {
-            System.out.println("Sound could not be loaded");
+            LOGGER.log(Level.WARNING, "Sound system unavailable, running without audio", e);
+            sound = null;
        }


When MIDI is unavailable you set sound = null, but other parts of the app dereference MainController.sound without null checks (e.g., Settings mute checkbox) and SortingSessionManager also calls sound.mute(...). This will cause NPEs on startup on systems without MIDI. Prefer assigning a no-op implementation (there is io.github.compilerstuck.Sound.HeadlessSound) so the rest of the code can treat sound as non-null.

Copilot · 2026-03-12T10:40:11Z

+    private boolean fullScreen = false;
+    private boolean portrait = false;

+    /**
+     * Entry point for the sorting visualizer application.
+     * 
+     * @param passedArgs command line arguments: "fullscreen" or "portrait"
+     */
    public static void main(String[] passedArgs) {
-        //Changing UI Theme
+        setupUITheme();
+
+        String[] appletArgs = new String[]{"io.github.compilerstuck.Control.MainController"};
+        PApplet.main(concat(appletArgs, passedArgs));
+    }


fullScreen/portrait are instance fields but never set from passedArgs anymore. This breaks the documented CLI behavior ("fullscreen" / "portrait") and makes settings() always use the standard window size. Consider parsing passedArgs in main and storing the result somewhere the sketch instance can read before settings() runs (e.g., static flags or a static copy of args that settings() checks).

Copilot · 2026-03-12T10:40:11Z

+     */
+    private void handleResultsDisplay() {
+        if (stateManager.shouldShowComparisonTable() && SortingAlgorithm.isRun()) {


handleResultsDisplay() only renders results when SortingAlgorithm.isRun() is true, but draw() prioritizes shouldShowResults() over shouldRestart(). If a run is cancelled (run becomes false) while stateManager.shouldShowResults() is true, the UI can get stuck in the results state and never reach restart handling. Consider clearing showResults (or falling through to restart) when SortingAlgorithm.isRun() is false, or conditioning setShowResults(true) on SortingAlgorithm.isRun() in the session manager.

Suggested change

*/

private void handleResultsDisplay() {

if (stateManager.shouldShowComparisonTable() && SortingAlgorithm.isRun()) {

*

* Ensures that if the sorting run is no longer active while the controller

* is in the "show results" state, the UI can transition to restart instead

* of remaining stuck in this branch.

*/

private void handleResultsDisplay() {

// If the algorithm is no longer running while we're in the results state,

// clear the results flag and request a restart so the draw loop can progress.

if (!SortingAlgorithm.isRun()) {

stateManager.setShowResults(false);

stateManager.setRestart(true);

return;

}

if (stateManager.shouldShowComparisonTable()) {

Copilot · 2026-03-12T10:40:11Z

+            for (SortingAlgorithm algorithm : algorithms) {
+                if (!stateManager.shouldContinueExecution()) {
+                    LOGGER.log(Level.INFO, "Sorting session cancelled by user");
+                    break;
+                }
+
+                recordTimestamp(startTime);
+                prepareForAlgorithm(algorithm);
+                executeAlgorithm(algorithm);
+
+                if (!stateManager.shouldContinueExecution()) {
+                    break;
+                }
+
+                recordMeasurements(algorithm);
+                pauseAfterAlgorithm();
+            }
+
+            if (stateManager.shouldContinueExecution() && stateManager.shouldShowComparisonTable()) {
+                stateManager.setShowResults(true);
+            }
+
+            stateManager.setRestart(true);
+        } catch (Exception e) {


SortingSessionManager uses stateManager.shouldContinueExecution() to detect cancellation, but the existing cancel action (Settings) toggles SortingAlgorithm.setRun(false) instead. As a result, cancelling may not break out of the session loop between algorithms and can incorrectly set showResults/restart states. To preserve existing behavior, also check SortingAlgorithm.isRun() in the loop and when deciding whether to show results (or alternatively wire the cancel button to stateManager.setContinueExecution(false)).

Copilot · 2026-03-12T10:40:12Z

+    private void prepareForAlgorithm(SortingAlgorithm algorithm) {
+        sound.mute(true);
+        sound.mute(false);
+
+        arrayController.shuffle();
+
+        if (!stateManager.shouldContinueExecution()) {
+            return;
+        }
+
+        sound.mute(true);
+        try {
+            Thread.sleep(MainControllerConfig.DELAY_BETWEEN_ALGORITHMS);
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+            LOGGER.log(Level.WARNING, "Thread interrupted during delay", e);
+        }
+        sound.mute(false);
+        arrayController.resetMeasurements();


sound can be null (e.g., when MIDI is unavailable in MainController.initializeComponents()), but this method calls sound.mute(...) unconditionally, which will crash the sorting thread with an NPE. Guard these calls with if (sound != null) (also in pauseAfterAlgorithm) or inject a no-op Sound implementation when audio is unavailable.

Copilot · 2026-03-12T10:40:12Z

+    /**
+     * Resets all state for a new sorting run.
+     */
+    public void resetForNewRun() {
+        userInitiatedStart.set(false);
+        isRunning.set(false);
+        showResults.set(false);
+        shouldRestart.set(false);
+        currentOperation = "Waiting";
+    }


The Javadoc says this "Resets all state for a new sorting run", but resetForNewRun() doesn't reset showComparisonTable, printMeasurements, or shouldContinueExecution. Either reset all fields to match the contract, or narrow the comment/name to reflect what is actually reset.

Copilot AI review requested due to automatic review settings March 12, 2026 10:34

Copilot started reviewing on behalf of 66-m March 12, 2026 10:34 View session

test(headless): initialize color gradient before visualization tests

c323bfc

Call updateGradient() to populate color array before passing to Bars and Circle visualizations. Fixes ArrayIndexOutOfBoundsException in CI test runs.

66-m merged commit 78ea2a3 into main Mar 12, 2026
2 checks passed

66-m deleted the refactor/main-controller-state-split branch March 12, 2026 10:37

Copilot AI reviewed Mar 12, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(control): split MainController state/session concerns#37

refactor(control): split MainController state/session concerns#37
66-m merged 2 commits into
mainfrom
refactor/main-controller-state-split

66-m commented Mar 12, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		* the Settings UI and algorithm infrastructure. Access to these is coordinated
		* through synchronized accessors.

	* Directfieldaccess to array controller.
	* Direct field access to array controller.

-     */
-    private void handleResultsDisplay() {
-        if (stateManager.shouldShowComparisonTable() && SortingAlgorithm.isRun()) {
+     *
+     * Ensures that if the sorting run is no longer active while the controller
+     * is in the "show results" state, the UI can transition to restart instead
+     * of remaining stuck in this branch.
+     */
+    private void handleResultsDisplay() {
+        // If the algorithm is no longer running while we're in the results state,
+        // clear the results flag and request a restart so the draw loop can progress.
+        if (!SortingAlgorithm.isRun()) {
+            stateManager.setShowResults(false);
+            stateManager.setRestart(true);
+            return;
+        }
+        if (stateManager.shouldShowComparisonTable()) {

Conversation

66-m commented Mar 12, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants