Skip to content

Conversation

IanMayo
Copy link
Contributor

@IanMayo IanMayo commented Aug 11, 2025

Summary

Removes the redundant state parameter from mode constructors since state is always accessible via instance.state.

Problem

The mode constructors were receiving both instance and state parameters, but state was always instance.state. This created:

  • Redundancy: Two ways to access the same state
  • Potential bugs: If someone passed a different state object
  • Confusion: Made it seem like state could be independent of instance
  • Unnecessary complexity: Extra parameter passing throughout the codebase

Solution

✅ Changes Made

  1. Updated BaseMode constructor

    • Removed state parameter
    • Now only accepts instance parameter
  2. Updated all mode constructors

    • AnalysisMode: Removed state param, 67 replacements
    • HarmonicsMode: Removed state param, 68 replacements
    • DopplerMode: Removed state param, 58 replacements
    • PanMode: Removed state param, 22 replacements
  3. Updated ModeFactory

    • createMode() now only passes instance
    • Updated error logging to check instance.state
  4. Updated ModeInitialization

    • Mode creation now only passes instance

Impact

Before:

constructor(instance, state) {
  super(instance, state)
  this.state = state  // Redundant - same as instance.state\!
}

After:

constructor(instance) {
  super(instance)
  // Access state via this.instance.state
}

Stats

  • 215 total replacements of this.state with this.instance.state
  • 5 constructor signatures simplified
  • 0 functional changes - pure refactoring

Benefits

  • 🎯 Eliminates redundancy - Single source of truth for state
  • 🛡️ Prevents bugs - No possibility of state mismatch
  • 📖 Clearer architecture - Obvious that state comes from instance
  • Simpler API - Mode creation only needs instance

Testing

✅ All 65 tests passing (2 skipped as before)
✅ TypeScript checking passes
✅ No functional changes - pure refactoring

Related

Continues the architectural improvements from:

This makes the codebase more maintainable and less error-prone.

Copy link

netlify bot commented Aug 11, 2025

Deploy Preview for gramframe ready!

Name Link
🔨 Latest commit a1f3cbc
🔍 Latest deploy log https://app.netlify.com/projects/gramframe/deploys/6899b4bc300f570008feb1e6
😎 Deploy Preview https://deploy-preview-147--gramframe.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@IanMayo IanMayo changed the base branch from main to issue-132-final-refactor August 11, 2025 09:16
@IanMayo IanMayo requested a review from Copilot August 11, 2025 09:17
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the mode system by removing the redundant state parameter from mode constructors since state is always accessible via instance.state. This eliminates potential bugs from state mismatch and simplifies the API by ensuring a single source of truth for state access.

Key Changes:

  • Updated all mode constructors to only accept instance parameter instead of both instance and state
  • Replaced all this.state references with this.instance.state throughout the mode classes
  • Updated ModeFactory and ModeInitialization to pass only the instance parameter

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/modes/BaseMode.js Updated base constructor signature to remove state parameter
src/modes/analysis/AnalysisMode.js Updated constructor and replaced 67 state references with instance.state
src/modes/harmonics/HarmonicsMode.js Updated constructor and replaced 68 state references with instance.state
src/modes/doppler/DopplerMode.js Updated constructor and replaced 58 state references with instance.state
src/modes/pan/PanMode.js Updated constructor and replaced 22 state references with instance.state
src/modes/ModeFactory.js Updated createMode method to only pass instance parameter
src/core/initialization/ModeInitialization.js Updated mode creation to only pass instance parameter

@IanMayo IanMayo merged commit e3661cb into issue-132-final-refactor Aug 11, 2025
4 checks passed
@IanMayo IanMayo deleted the refactor-remove-redundant-state-param branch August 11, 2025 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant