Skip to content

refactor: Improve base Modifier class design #187

@andeplane

Description

@andeplane

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:

  1. Make run abstract (if TypeScript supports abstract methods in classes)
  2. OR add clear JSDoc documentation explaining that subclasses must override
  3. 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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions