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
run abstract (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 - implements run
SyncBondsModifier - implements run
ColorModifier - implements run
SyncComputesModifier - implements run
SyncFixesModifier - implements run
SyncVariablesModifier - implements run
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
Problem
The base
Modifierclass has an emptyrun()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.tscontains:This empty implementation:
Expected Behavior
Either:
runabstract (if TypeScript supports abstract methods in classes)Investigation Needed
Before implementing, check if any existing modifiers rely on the empty implementation:
SyncParticlesModifier- implementsrunSyncBondsModifier- implementsrunColorModifier- implementsrunSyncComputesModifier- implementsrunSyncFixesModifier- implementsrunSyncVariablesModifier- implementsrunAll seem to override it, so abstract method might be safe.
Files to Modify
src/modifiers/modifier.tsProposed Solution
If all modifiers override
run, make it abstract:If TypeScript doesn't support abstract methods in this context, add clear JSDoc:
Benefits