Skip to content

[Refactor]: Rename loop indices outside the short-loop exception in capture.ts and is/index.ts #75

Description

@martyy-code

Current State

In packages/errors/src/is/index.ts:104 and packages/errors/src/error/capture.ts:22, 30, the loop index i is used inside functions that are longer than the "tight loop" exception that rule 0005 allows:

  • is/index.ts:104for (let i = 0; i < inherits.length; i = i + 1) inside a function body of 60+ lines, with two nested control structures (a while loop and a Set membership check) above it.
  • capture.ts:22, 30 — two separate for loops over lines, each six lines long, in a 25-line function.

Rule 0005 states:

"Loop indices (i, j, k) are the single exception because they are a mathematical convention, not a project choice. The exception is scoped to tight, single-screen loops where the convention is universal. An i in a fifty-line function is not the same as an i in a five-line loop; the second is convention, the first is a diminished name that should be spelled out."

The current usage is the first kind, not the second.

Located in:

  • packages/errors/src/is/index.ts:104
  • packages/errors/src/error/capture.ts:22
  • packages/errors/src/error/capture.ts:30

Proposed State

After refactoring, the loop indices carry their concept:

// capture.ts:22 — finding the start of stack frames
for (let frameIndex = 0; frameIndex < lines.length; frameIndex += 1) {
  if (STACK_FRAME_PATTERN.test(lines[frameIndex])) {
    startIndex = frameIndex;
    break;
  }
}

// capture.ts:30 — filtering out internal frames
for (let lineIndex = startIndex; lineIndex < lines.length; lineIndex += 1) {
  const line = lines[lineIndex];
  // ...
}

// is/index.ts:104 — pushing parents to the inheritance walk stack
for (let parentIndex = 0; parentIndex < inherits.length; parentIndex += 1) {
  stack.push(inherits[parentIndex]);
}

A more aggressive refactor (consistent with rule 0007 — top-down composition) extracts the start-finding loop into a named function:

const findFirstFrameIndex = (lines: string[]): number => {
  for (let index = 0; index < lines.length; index += 1) {
    if (STACK_FRAME_PATTERN.test(lines[index])) {
      return index;
    }
  }
  return 0;
};

Expected improvements:

  • Rule 0005 compliance: the index is no longer a diminished name.
  • Rule 0007 bonus: the search becomes a named operation the caller can read in one line.
  • The intent of each loop is visible from the first line of the body, not after the reader parses a counter.

Motivation

This refactoring is needed because:

  • The current i is a project-internal diminutive per rule 0005, used outside the loop-convention exception.
  • The function bodies are long enough that the reader has to keep i in their head for 20+ lines; spelling the index removes that tracking cost.
  • Extracting the search into a named function aligns with rule 0007 and is a small, local change.

Triggers for this work:

  • Technical debt accumulation
  • Maintainability concerns

Risks

Potential risks:

  • Risk 1: Renaming i to a longer name in a hot path might affect performance. — Mitigation: V8 inlines and renames local variables freely; the source-level rename has no runtime cost. The change is also not in a benchmarked hot path.
  • Risk 2: Extracting findFirstFrameIndex changes the call shape. — Mitigation: the function is a pure utility with no closure dependencies; extraction is mechanical.

Migration Plan

Migration approach:

  1. Rename the indices in capture.ts to frameIndex and lineIndex.
  2. (Optional) Extract findFirstFrameIndex into a named function inside capture.ts.
  3. Rename the index in is/index.ts:104 to parentIndex.
  4. Run the test suite.

Rollback plan: revert the PR.

Backward Compatibility

  • This refactoring maintains full backward compatibility

Scope

Files/Folders affected:

  • packages/errors/src/is/index.ts (line 104)
  • packages/errors/src/error/capture.ts (lines 22, 30)

Component(s) Affected

  • Multiple Components

Note: the component_affected dropdown is calibrated for a web template project. The actual affected component is packages/errors.

Priority

  • p0: Critical - Blocking major work or causing bugs
  • p1: High - Important, should do soon
  • p2: Medium - Normal priority
  • p3: Low - Nice to have

Estimated Effort

  • effort: xs - Few minutes

Test Coverage Requirements

  • Existing tests cover this code area (will update)

Testing Approach

Verification steps:

  1. pnpm --filter @deessejs/errors test:run
  2. pnpm --filter @deessejs/errors type-check

Related Issues / Pull Requests

  • Related audit: P0 feat: implement causes() function for cause chain traversal #5 in the internal audit of packages/errors/src/ against rules 0001-0016, August 2026.
  • Related rule: docs/engineering/architecture/rules/0005-named-algorithms-and-independent-data-structures.md.
  • Related rule: docs/engineering/architecture/rules/0007-top-down-composition.md.

Relevant Documentation

  • Architecture doc: docs/engineering/architecture/rules/0005-named-algorithms-and-independent-data-structures.md

Pre-Submission Checklist

  • I have searched existing issues for related refactoring requests
  • Risks and migration plan are documented
  • Test coverage approach is defined
  • I understand this issue will be labeled according to the project taxonomy
  • This is NOT a security vulnerability (see security note above)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions