Step by step api clean by Adish-Singh · Pull Request #179 · matthew-SG/CSC207_Project

Adish-Singh · 2025-12-03T00:36:10Z

Summary
This pull request introduces full text-to-speech (TTS) support to the step-by-step recipe flow using a new SpeechService abstraction and a concrete SystemTTS implementation. In addition, it applies substantial refactoring across interactors, presenters, state objects, and view models to restore clean architectural boundaries and ensure consistent and testable behavior.The PR also resolves several defects, removes obsolete code, and adds a complete test suite for the navbar interactor.

Key Changes

SystemTTS Enhancements

Updated synthesize to throw Exception instead of silently failing.
Added validation for empty/invalid input.
Unified unsupported OS handling by throwing an exception.
Ensured calling layers can detect and handle TTS errors.

SpeechService Interface Updates

Consolidated to a single method: void synthesize(String text) throws Exception.
Removed the previous duplicate method definition.

Step-By-Step Module Refactor and TTS Integration

Controller : Added speak() method to trigger TTS through the interactor.
Interactor:
- Added SpeechService dependency.
- Added executeSpeak(StepByStepInputData) for TTS operations.
- Executes TTS asynchronously in a background thread.
- Improved validation and error handling.
- Refactored navigation logic for clarity and correctness.
Presenter:

Reworked presenter methods:
prepareSuccessView
prepareFailView
prepareSpeakFailView
Ensures error messages and speaking state are properly reflected in the view model.

StepByStepState

Removed duplicated fields and methods.
Added new fields: isSpeaking, errorMessage
Updated documentation and simplified getters/setters.

StepByStepViewModel

Unified property change mechanism (firePropertyChanged).
Simplified listener management.
Removed obsolete methods and duplicate logic.

StepByStepView

Removed direct TTS invocation from the view.
View now triggers the controller’s speak() method following Clean Architecture.
Updated property change handling to support both state updates and error notifications.

Additional Fixes

LikedRecipeListPresenter

Replaced outdated method calls:
instructions() → getInstructions().
Removed redundant TTS initialization.
Fixed incorrect presenter reference.

RecipeInstructions (Minor cleanup.)

Tests

LikedRecipeInteractorTest

Updated to use getInstructions() instead of the removed instructions() accessor.

Added: Navbar Tests

New files:
- NavbarInteractorTests
- NavbarMockPresenter
Covers:
- All navigation routes
- Sequential navigation calls
- Correct delegation to presenter

Rationale
Brings the step-by-step module back into alignment with Clean Architecture.
Introduces consistent and testable TTS behavior.
Reduces duplication, improves maintainability, and fixes existing defects.
Adds missing tests for key navigation functionality.

Impact
No external API changes outside the step-by-step use case.
All existing UI components remain functional with improved consistency and reliability.

…enter file since navbar does not have anything that implements its output boundary in its use case, i.e. it does not have its own presenter.

…dhere to clean architecture. So, had to change some of the test files as well

github-actions

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

checkstyle

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.imports.RedundantImportCheck> _{reported by reviewdog 🐶}
Redundant import from the same package - use_case.nav_bar.NavbarOutputBoundary.

CSC207_Project/src/test/java/use_case/nav_bar/NavbarMockPresenter.java

Line 3 in 3fdad81

import use_case.nav_bar.NavbarOutputBoundary;

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.coding.ExplicitInitializationCheck> _{reported by reviewdog 🐶}
Variable 'lastViewSwitched' explicitly initialized to 'null' (default value for its type).

CSC207_Project/src/test/java/use_case/nav_bar/NavbarMockPresenter.java

Line 11 in 3fdad81

private String lastViewSwitched = null;

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.naming.PackageNameCheck> _{reported by reviewdog 🐶}
Name 'use_case.nav_bar' must match pattern '^[a-z]+(.[a-zA-Z_]\w*)*$'.

CSC207_Project/src/test/java/use_case/nav_bar/NavbarInteractorTests.java

Line 1 in 3fdad81

package use_case.nav_bar;

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.imports.CustomImportOrderCheck> _{reported by reviewdog 🐶}
Import statement for 'org.junit.jupiter.api.Assertions.*' is in the wrong order. Should be in the 'STATIC' group, expecting not assigned imports on this line.

CSC207_Project/src/test/java/use_case/nav_bar/NavbarInteractorTests.java

Line 5 in 3fdad81

import static org.junit.jupiter.api.Assertions.*;

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.imports.ImportOrderCheck> _{reported by reviewdog 🐶}
Wrong order for 'org.junit.jupiter.api.Assertions.*' import.

CSC207_Project/src/test/java/use_case/nav_bar/NavbarInteractorTests.java

Line 5 in 3fdad81

import static org.junit.jupiter.api.Assertions.*;

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.imports.AvoidStarImportCheck> _{reported by reviewdog 🐶}
Using the '.' form of import should be avoided - org.junit.jupiter.api.Assertions..

CSC207_Project/src/test/java/use_case/nav_bar/NavbarInteractorTests.java

Line 5 in 3fdad81

import static org.junit.jupiter.api.Assertions.*;

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck> _{reported by reviewdog 🐶}
Variable 'instructions' should be declared final.

CSC207_Project/src/test/java/use_case/liked_recipe_list/LikedRecipeInteractorTest.java

Line 412 in 3fdad81

    
           RecipeInstructions instructions = presenter.lastHandsfreeInput.getInstructions();

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.coding.IllegalCatchCheck> _{reported by reviewdog 🐶}
Catching 'Exception' is not allowed.

CSC207_Project/src/main/java/use_case/step_by_step/StepByStepInteractor.java

Line 78 in 3fdad81

catch (Exception exception) {

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck> _{reported by reviewdog 🐶}
Variable 'speechService' should be declared final.

CSC207_Project/src/main/java/interface_adapter/liked_recipe_list/LikedRecipeListPresenter.java

Line 64 in 3fdad81

SpeechService speechService = new SystemTTS();

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck> _{reported by reviewdog 🐶}
Variable 'interactor' should be declared final.

CSC207_Project/src/main/java/interface_adapter/liked_recipe_list/LikedRecipeListPresenter.java

Line 67 in 3fdad81

    
           StepByStepInteractor interactor = new StepByStepInteractor(stepPresenter, speechService);

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.naming.LambdaParameterNameCheck> _{reported by reviewdog 🐶}
Name 'e' must match pattern '^(id)|([a-z][a-z0-9][a-zA-Z0-9]+)$'.

CSC207_Project/src/main/java/view/StepByStepView.java

Line 102 in 3fdad81

speakButton.addActionListener(e -> stepByStepController.speak());

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck> _{reported by reviewdog 🐶}
Variable 'propertyName' should be declared final.

CSC207_Project/src/main/java/view/StepByStepView.java

Line 107 in 3fdad81

String propertyName = evt.getPropertyName();

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck> _{reported by reviewdog 🐶}
Variable 'newState' should be declared final.

CSC207_Project/src/main/java/view/StepByStepView.java

Line 110 in 3fdad81

StepByStepState newState = (StepByStepState) evt.getNewValue();

🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck> _{reported by reviewdog 🐶}
Variable 'newState' should be declared final.

CSC207_Project/src/main/java/view/StepByStepView.java

Line 114 in 3fdad81

StepByStepState newState = (StepByStepState) evt.getNewValue();

github-actions · 2025-12-03T00:36:42Z

-        assertEquals(1, output.stepNumber());
-        assertTrue(output.hasNext());
-        assertFalse(output.hasPrevious());
+        assertEquals("Heat the oven to 350°F", output.getStepText());


🚫 [checkstyle] _{reported by reviewdog 🐶}
Line matches the illegal pattern '[^\p{ASCII}]'.

github-actions · 2025-12-03T00:36:43Z

        // Arrange
-        RecipeInstructions emptyInstructions = new RecipeInstructions(Collections.emptyList());
-        StepByStepInputData inputData = new StepByStepInputData(emptyInstructions, 0);
+        RecipeInstructions instructions = new RecipeInstructions(sampleSteps);


🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck> _{reported by reviewdog 🐶}
Variable 'instructions' should be declared final.

github-actions · 2025-12-03T00:36:43Z

-        RecipeInstructions emptyInstructions = new RecipeInstructions(Collections.emptyList());
-        StepByStepInputData inputData = new StepByStepInputData(emptyInstructions, 0);
+        RecipeInstructions instructions = new RecipeInstructions(sampleSteps);
+        StepByStepInputData inputData = new StepByStepInputData(instructions, 2);


🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck> _{reported by reviewdog 🐶}
Variable 'inputData' should be declared final.

github-actions · 2025-12-03T00:36:43Z

-        assertFalse(mockPresenter.isPresentCalled(), "Present should not be called for empty steps");
+        assertTrue(mockPresenter.isSuccessViewCalled());
+
+        StepByStepOutputData output = mockPresenter.getLastOutputData();


🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck> _{reported by reviewdog 🐶}
Variable 'output' should be declared final.

github-actions · 2025-12-03T00:36:43Z

        // Arrange
-        RecipeInstructions nullInstructions = new RecipeInstructions(null);
-        StepByStepInputData inputData = new StepByStepInputData(nullInstructions, 0);
+        RecipeInstructions instructions = new RecipeInstructions(sampleSteps);


🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck> _{reported by reviewdog 🐶}
Variable 'instructions' should be declared final.

github-actions · 2025-12-03T00:36:45Z

+    /**
+     * Mock speech service for testing
+     */
+    private static class MockSpeechService implements SpeechService {


🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.design.FinalClassCheck> _{reported by reviewdog 🐶}
Class MockSpeechService should be declared as final.

github-actions · 2025-12-03T00:36:45Z

+     * Mock speech service for testing
+     */
+    private static class MockSpeechService implements SpeechService {
+        private boolean synthesizeCalled = false;


🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.coding.ExplicitInitializationCheck> _{reported by reviewdog 🐶}
Variable 'synthesizeCalled' explicitly initialized to 'false' (default value for its type).

github-actions · 2025-12-03T00:36:45Z

+     */
+    private static class MockSpeechService implements SpeechService {
+        private boolean synthesizeCalled = false;
+        private String lastTextSpoken = null;


🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.coding.ExplicitInitializationCheck> _{reported by reviewdog 🐶}
Variable 'lastTextSpoken' explicitly initialized to 'null' (default value for its type).

github-actions · 2025-12-03T00:36:46Z

+    private static class MockSpeechService implements SpeechService {
+        private boolean synthesizeCalled = false;
+        private String lastTextSpoken = null;
+        private boolean shouldFail = false;


🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.coding.ExplicitInitializationCheck> _{reported by reviewdog 🐶}
Variable 'shouldFail' explicitly initialized to 'false' (default value for its type).

github-actions · 2025-12-03T00:36:46Z

@@ -0,0 +1,70 @@
+package use_case.nav_bar;


🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.naming.PackageNameCheck> _{reported by reviewdog 🐶}
Name 'use_case.nav_bar' must match pattern '^[a-z]+(.[a-zA-Z_]\w*)*$'.

matthew-SG

I like the changes you made. Since the checkstyle issues appear to be mainly for your tests, I'll approve the merge.

taahacodes

I see you added more test cases which is good, please address the checkstyle issues though. I noticed your step by step interactor assumes steps is never empty or null, is this always the case? Because if it is empty or the index is out of range it'll throw before the fail view is reached you should probably add these edge cases in your tests too. Also i wanted to mention in the StepbyStep presenter an and the view model both have different fail views with very similar behaviour it might be better to merge these into a single error path.

hussssni

This is the best code I've ever seen

hussssni

Spectacular

matthew-SG · 2025-12-03T01:41:54Z

We have three approvals, I'm vetoing it.

Adish-Singh added 3 commits December 2, 2025 18:34

Created the tests for the navbar interactor. Had to create a mockpres…

cfcbf84

…enter file since navbar does not have anything that implements its output boundary in its use case, i.e. it does not have its own presenter.

Fixed the architecture for the API call to get the steps, making it a…

2135e9f

…dhere to clean architecture. So, had to change some of the test files as well

Fixed checkstyle issues

3fdad81

github-actions Bot reviewed Dec 3, 2025

View reviewed changes

matthew-SG approved these changes Dec 3, 2025

View reviewed changes

taahacodes approved these changes Dec 3, 2025

View reviewed changes

hussssni approved these changes Dec 3, 2025

View reviewed changes

matthew-SG merged commit d23fd82 into main Dec 3, 2025
1 check passed

Conversation

Adish-Singh commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

matthew-SG left a comment

Choose a reason for hiding this comment

Uh oh!

taahacodes left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hussssni left a comment

Choose a reason for hiding this comment

Uh oh!

hussssni left a comment

Choose a reason for hiding this comment

Uh oh!

matthew-SG commented Dec 3, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Adish-Singh commented Dec 3, 2025 •

edited

Loading

taahacodes left a comment •

edited

Loading