Repository navigation
refactor: replace program/target/ion magic numbers with named constants #114
Description
Part of #108 (health-review epic).
Problem
Several semantically meaningful thresholds appear as bare literals scattered across src/dedx.c and src/dedx_validate.c:
program >= 100— the "analytical program" threshold (Bethe/default).target > 99— the "custom compound id" threshold.ion > 120/i < 112— maximum atomic number / ion-table extent.
These are easy to get subtly wrong (note > 99 vs >= 100, and 112 vs 113 buffer sizing) and obscure intent.
Proposed change
Introduce documented named constants (e.g. DEDX_ANALYTICAL_PROGRAM_MIN, DEDX_COMPOUND_ID_MIN, DEDX_MAX_ELEMENT_Z) in a central internal header and use them everywhere the literals currently appear. No behavior change.
Acceptance criteria
- The literals
100/99/120/112used as these thresholds are replaced by named constants with explanatory comments at their definition. - No behavioral change — full ctest suite still passes.
- A single source of truth defines the max-Z / table extent used for both the loop bound and any buffer sizing.
Activity
grzanka commentedon Aug 8, 2026
Phase 0 of #149 (#150, merged) added three more instances of the exact
112/113duplication this issue is about, in a file this issue's "Problem" section doesn't mention yet — worth folding into the fix when this is picked up:src/dedx_periodic_table.c:8,17,25—dedx_internal_get_atom_charge()/get_atom_mass()/get_nucleon()now each doif (id >= 1 && id < 113). These are the lowest-level lookups (they indexdedx_amu[]/dedx_nucl[]directly), so they're arguably the best home for the single source of truth this issue asks for, rather thandedx.c's private#define DEDX_MAX_ELEMENT_Z 112(src/dedx.c:198, added just before Phase 0 of #149: docs and guards, no behaviour change #150 in d5bf0cd) — that macro isn't visible fromdedx_periodic_table.c, which is exactly why Phase 0 of #149: docs and guards, no behaviour change #150 had to reach for a raw113instead of reusing it.src/dedx_program_const.h:66—dedx_full_ion_list[113], whose own comment already says "The count 112 matches the periodic-table arrays dedx_amu[] / dedx_nucl[] in dedx_periodic_table.h; keep them in sync if elements are ever added" — i.e. a fourth hand-synced copy of the same number.src/dedx.c:626—if ((ion < 1) || (ion > 112)), already listed in this issue's Problem section but confirming it's still present post-Phase 0 of #149: docs and guards, no behaviour change #150.Suggested single source of truth: define the constant once in
src/dedx_periodic_table.h(next todedx_amu[112]/dedx_nucl[112], which are the actual data it's describing) as e.g.#define DEDX_MAX_ELEMENT_Z 112, sizededx_amu/dedx_nucl/dedx_full_ion_listoff it (dedx_full_ion_list[DEDX_MAX_ELEMENT_Z + 1]), and havededx.cinclude and reuse it instead of redefining it locally.(Checked the rest of the codebase for the same pattern while at it — the
98/99element/compound boundary this issue also lists is additionally tracked as a correctness bug, not just a naming one, in #149's finding A2; the various hand-maintained table sizes indedx_program_const.h(110,120,290,1000,40) are tracked separately in #149's B3/D4 and #138.DEDX_MAX_ELEMENTS(150, indedx_elements.h) is already a proper shared named constant used consistently everywhere — no issue there. Didn't find any other undocumented duplicated-literal families beyond what's already listed here and in #149.)Generated by Claude Code