Skip to content

refactor: replace magic numbers with named constants (#88)#142

Merged
s2x merged 1 commit into
mainfrom
feat/smell-007-magic-numbers
May 30, 2026
Merged

refactor: replace magic numbers with named constants (#88)#142
s2x merged 1 commit into
mainfrom
feat/smell-007-magic-numbers

Conversation

@s2x

@s2x s2x commented May 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Replaces all magic number literals throughout the PHP codebase with named constants for improved readability and maintainability.

Changes

New Constants in class

  • (64 MB) — default collection buffer size
  • — initial schema string buffer
  • — initial path/stats string buffer
  • — 1 MB max for string buffers
  • — memory limit conversion factor
  • — default HNSW graph connectivity
  • — default HNSW build quality

Files Modified

  • — replaced 10+ magic number instances with constants
  • — updated and defaults

Tests

  • — verifies all constants have expected values and no magic numbers remain in source

Verification

All 128 tests pass (125 passed, 2 expected failures, 1 skipped).

C++ Layer

The switch statements already use proper C++ enum values (, , etc.) — no changes needed there.

- Add DEFAULT_MAX_BUFFER_SIZE, SCHEMA_BUFFER_SIZE, PATH_BUFFER_SIZE,
  MAX_STRING_BUFFER_SIZE, BYTES_PER_MB constants to ZVec class
- Add DEFAULT_HNSW_M and DEFAULT_HNSW_EF_CONSTRUCTION constants
- Replace all magic number literals in ZVec.php with constant references
- Update ZVecIndexParams.php to use ZVec::DEFAULT_HNSW_M and
  ZVec::DEFAULT_HNSW_EF_CONSTRUCTION
- Add test_magic_number_constants.phpt verifying all constants and
  that magic numbers are eliminated from source

Fixes #88
@s2x s2x force-pushed the feat/smell-007-magic-numbers branch from 5f75d83 to 5a0d7db Compare May 30, 2026 07:29
@s2x s2x merged commit f139e36 into main May 30, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant