-
Notifications
You must be signed in to change notification settings - Fork 24
Open
Description
[Architecture Proposal] Refactoring Instruction class to improve decoupling and extensibility
Context
After analyzing the project structure and core logic, specifically within the LadderApp/Model/Instructions/ directory, I’ve identified several areas where refactoring could align with the project's roadmap goals, such as "Big code refactoring" and "Decoupling the ladder language processor".
Observed Issues
- Single Responsibility Principle (SRP) Violation: The
Instruction.csclass currently manages base properties, XML serialization, and complex validation logic. - Maintenance Complexity: The
ValidateAddressmethod uses a largeswitch-caseblock to handle different instruction types (Timers, Counters, Coils). This makes the base class fragile and violates the Open/Closed Principle, as adding new instructions requires modifying the base class. - Redundant Code: There is a significant amount of commented-out logic in
Instruction.cswhich impacts code readability. - Interface Utilization: The
IInstructioninterface is currently thin. Moving more behavioral definitions (like validation) here would improve polymorphism.
Proposed Solution
- Decouple Validation: Move the logic currently inside
ValidateAddressinto specific subclasses (e.g.,TimerInstruction,CounterInstruction) by overriding a virtual validation method. - Lean Base Class: Refactor
Instruction.csto hold only common metadata and delegate specific logic to derived classes. - Enhance IInstruction: Update the interface to include a
Validate()contract that all instructions must fulfill.
Impact
This refactoring is a prerequisite for successfully extending the base code to other platforms like Arduino, PIC, or STM32, as it will make the logic processor platform-agnostic and easier to test.
I am interested in contributing to this project. If you agree with this direction, I can prepare a Pull Request for these changes.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels