-
-
Notifications
You must be signed in to change notification settings - Fork 9
Open
Description
Problem
The base Modifier class has an empty run() method implementation that is not abstract. This creates ambiguity about whether the method should be overridden and makes it easy to forget to implement it in subclasses.
Current Behavior
src/modifiers/modifier.ts contains:
run = (
input: ModifierInput,
output: ModifierOutput,
everything: boolean = false,
) => {};This empty implementation:
- Doesn't clearly indicate that subclasses must override it
- No TypeScript enforcement to ensure implementation
- No documentation explaining the pattern
Expected Behavior
Either:
- Make
runabstract (if TypeScript supports abstract methods in classes) - OR add clear JSDoc documentation explaining that subclasses must override
- Consider if any modifiers actually need an empty implementation (should check all existing modifiers first)
Investigation Needed
Before implementing, check if any existing modifiers rely on the empty implementation:
SyncParticlesModifier- implementsrunSyncBondsModifier- implementsrunColorModifier- implementsrunSyncComputesModifier- implementsrunSyncFixesModifier- implementsrunSyncVariablesModifier- implementsrun
All seem to override it, so abstract method might be safe.
Files to Modify
src/modifiers/modifier.ts
Proposed Solution
If all modifiers override run, make it abstract:
abstract run(
input: ModifierInput,
output: ModifierOutput,
everything?: boolean
): void;If TypeScript doesn't support abstract methods in this context, add clear JSDoc:
/**
* Processes the modifier logic. Must be overridden by subclasses.
* @param input - The modifier input containing WASM, LAMMPS, and state data
* @param output - The modifier output to populate
* @param everything - Whether to process everything or only changes
*/
run = (
input: ModifierInput,
output: ModifierOutput,
everything: boolean = false,
) => {
throw new Error('run() must be implemented by subclass');
};Benefits
- Clearer API contract
- Better developer experience (TypeScript errors if not implemented)
- Self-documenting code
Metadata
Metadata
Assignees
Labels
No labels