#41 Motor ZPP refactor

[EddieMachete]

Closes #41

Issues and suggestions

1. Bug: mutation in sortMotorZPP

// motorZPP.ts - Lines 151-155
export function sortMotorZPP(state: State): Step { 
  const zpp = state.zpp.sort((a, b) => {
    const aIndex = a === 'NA' ? -1 : SensoryLevels.indexOf(a.replace(/\*/, '') as SensoryLevel);
    const bIndex = b === 'NA' ? -1 : SensoryLevels.indexOf(b.replace(/\*/, '') as SensoryLevel);
    return aIndex - bIndex;

Array.prototype.sort() mutates the array. This breaks immutability and can cause subtle bugs.
Fix: Sort a copy of the array:

const zpp = [...state.zpp].sort((a, b) => {

2. hasStarOnCurrentOrAboveLevel – redundant logic

// motorZPP.ts - Lines 123-141

function hasStarOnCurrentOrAboveLevel(currentLevel: SideLevel, lastLevelWithConsecutiveNormalValues: SideLevel, firstLevelWithStar: SideLevel | null): boolean {
  // Good example, case #93
  if (!firstLevelWithStar) {
    return false;
  }

  if (currentLevel.motor !== null) {
    return /0\*/.test(currentLevel.motor);
  }

  if (/\d\*/.test(currentLevel.lightTouch) || /\d\*/.test(currentLevel.pinPrick)) {
    return true;
  }

  // return currentLevel.index <= lastLevelWithConsecutiveNormalValues.index && currentLevel.index >= firstLevelWithStar.index;
  return /\d\*/.test(currentLevel.lightTouch)
    || /\d\*/.test(currentLevel.pinPrick)
    || (currentLevel.index <= lastLevelWithConsecutiveNormalValues.index && currentLevel.index >= firstLevelWithStar.index);
}

• The /\d*/.test(...) checks on lines 137–138 are unreachable because the same condition returns earlier on lines 133–135.
• The comment // Good example, case #93 is vague.
• The commented-out line should be removed.

Suggested refactor:

function hasStarOnCurrentOrAboveLevel(
  currentLevel: SideLevel,
  lastLevelWithConsecutiveNormalValues: SideLevel,
  firstLevelWithStar: SideLevel | null): boolean {
  if (!firstLevelWithStar) return false;

  if (currentLevel.motor !== null) {
    return /0\*/.test(currentLevel.motor);
  }

  const hasStarOnCurrentLevel = /\d\*/.test(currentLevel.lightTouch) || /\d\*/.test(currentLevel.pinPrick);
  if (hasStarOnCurrentLevel) return true;

  const isInStarRange =
    currentLevel.index >= firstLevelWithStar.index &&
    currentLevel.index <= lastLevelWithConsecutiveNormalValues.index;  return isInStarRange;
}

3. Extract regex and magic values

Patterns like /(^2$)|(\*\*$)/, /(^5$)|(\*\*$)/, /^[1-5]/, /^(NT|[0-4])\*\*$/ are hard to read and reuse.

Suggestion: Move them to named constants or small helpers:

// At top of file or in a shared constants module
const MOTOR_PATTERNS = {
  NORMAL_MOTOR: /^[1-5]/,
  NORMAL_WITH_VARIABLE: /^(NT|[0-4])\*\*$/,
  NOT_TESTED_OR_ZERO_STAR: /^(NT\*?$)|(0\*$)/,
} as const;

const SENSORY_PATTERNS = {
  NORMAL: /(^2$)|(\*\*$)/,
} as const;

Then use MOTOR_PATTERNS.NORMAL_MOTOR.test(...) etc. This improves readability and makes changes easier.

4. Large step functions

checkForMotorFunction and checkForSensoryFunction are long and branchy. Consider:

• Extracting helpers for building Step objects.
• Extracting predicates for conditions like overrideWithNonKeyMuscle, isTopRange, etc.
• Using early returns to reduce nesting.
  Example for checkForMotorFunction:

// Extract predicate
function shouldOverrideWithNonKeyMuscle(state: State, currentLevel: SideLevel): boolean {
  return Boolean(
    state.testNonKeyMuscle &&
    state.nonKeyMuscle &&
    state.nonKeyMuscle.index - currentLevel.index > 3  );
}

// Extract step builder
function createStep(description: Step['description'], actions: Step['actions'], state: State, next: Step['next']): Step {
  return { description, actions, state, next };
}

5. getLevelsRange complexity

getLevelsRange does several things in one loop:

• Builds the level chain
• Finds firstLevelWithStar
• Finds lastLevelWithConsecutiveNormalValues
• Finds nonKeyMuscle
• Finds topLevel and bottomLevel

Suggestion: Split into smaller functions, e.g.:

• buildLevelChain(side, top, bottom)
• findFirstLevelWithStar(levels)
• findLastLevelWithConsecutiveNormalValues(levels, bottom)
  Then compose them in getLevelsRange. This makes the logic easier to follow and test.

6. determineMotorZPP entry point

let step: Step = {
  description: {key: 'motorZPPCheckIfMotorZPPIsApplicableDescription'},
  actions: [],
  state: getInitialState(side, voluntaryAnalContraction, ais, motorLevel),
  next: checkIfMotorZPPIsApplicable,
};

The initial step is synthetic and only used to bootstrap the loop. A clearer approach is to start from the first real step:

export function determineMotorZPP(
  side: ExamSide,
  voluntaryAnalContraction: BinaryObservation,
  ais: string,
  motorLevel: string
): string {
  const initialState = getInitialState(side, voluntaryAnalContraction, ais, motorLevel);
  let step = checkIfMotorZPPIsApplicable(initialState);

  while (step.next) {
    step = step.next(step.state);
  }

  return step.state.zpp.join(',');
}

This removes the synthetic step and makes the flow more obvious.

7. Step iterator for step-by-step execution

To support stepping through the calculation, expose a generator or iterator:

export function* motorZPPSteps(
  side: ExamSide,
  voluntaryAnalContraction: BinaryObservation,
  ais: string,
  motorLevel: string
): Generator<Step> {
  const initialState = getInitialState(side, voluntaryAnalContraction, ais, motorLevel);
  let step = checkIfMotorZPPIsApplicable(initialState);

  yield step;

  while (step.next) {
    step = step.next(step.state);
    yield step;
  }
}

Consumers can then iterate step by step or collect all steps.

8. Error handling

Errors like 'state.currentLevel is null. A SideLevel value is required.' are clear. Consider:

• Custom error types for domain errors.
• Centralizing error messages (e.g. constants or a small error module) for consistency and i18n.

9. Type for next

next: ((state: State) => Step) | null is correct but a bit opaque. A type alias can make the API clearer:

export type StepHandler = (state: State) => Step;

export type Step = {
  description: { key: Translation; params?: { [key: string]: string } };
  actions: { key: Translation; params?: { [key: string]: string } }[];
  next: StepHandler | null;
  state: State;
};

[EddieMachete]

Apologies for so many changes. Most of them are formatting as I've been letting my IDE auto-format on save based on the prettier rules.

[EddieMachete]

Bug: mutation in sortMotorZPP
Array.prototype.sort() mutates the array. This breaks immutability and can cause subtle bugs.

Fix: Sort a copy of the array:
const zpp = [...state.zpp].sort((a, b) => {

[EddieMachete]

The /\d\*/.test(...) checks were unreachable because the same condition was being returned earlier.

[Copilot]

Pull request overview

This pull request implements a comprehensive refactoring of the Motor ZPP (Zone of Partial Preservation) calculation module, addressing all 7 objectives outlined in issue #41. The refactoring improves code maintainability, readability, and testability while preserving all existing functionality.

Changes:

• Introduced custom error handling with MotorZPPError class and centralized error messages
• Extracted regex patterns and magic values into a well-documented PATTERNS constant object
• Decomposed large functions (getLevelsRange, checkForSensoryFunction, checkForMotorFunction) into smaller, focused helper functions
• Added motorZPPSteps generator function for step-by-step execution visibility
• Fixed mutation bug in sortMotorZPP by using array spread operator
• Added type aliases (StepHandler) and helper functions (createStep, buildMotorZPPLevelName) to reduce code duplication
• Updated development tooling (prettier, eslint) and removed unused test code

Reviewed changes

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

Show a summary per file

File	Description
src/classification/zoneOfPartialPreservation/motorZPPErrors.ts	New file introducing custom error class with typed error codes for better error handling
src/classification/zoneOfPartialPreservation/motorZPP.ts	Major refactoring: extracted constants, decomposed functions, added generator, improved maintainability
src/classification/zoneOfPartialPreservation/motorZPP.spec.ts	Updated tests for new error types and added comprehensive tests for motorZPPSteps generator
src/classification/commonSpec.ts	Added explicit void return types to helper functions
src/classification/asiaImpairmentScale/asiaImpairmentScale.spec.ts	Removed unused imports and variables
package.json	Updated eslint version and added prettier; modified test script for explicit file patterns
package-lock.json	Dependency updates reflecting package.json changes
.prettierrc	New prettier configuration file with singleQuote preference
.eslintrc.json	Reformatted for consistency and added output directories to ignorePatterns

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.