Skip to content

Design review report #10

@kdw503

Description

@kdw503

Design Review — RegisterUtilities

Date: 2026-04-29

Likely Design Issues

1. Testing utilities exported alongside production code

quadratic, block_center, and tighten are described as "testing-support
helpers originally extracted from RegisterTestUtilities," yet they are expo
rted from the main package at the same level as Counter. Every package dep
ending on RegisterUtilities for Counter silently gets a dependency on R egisterCore (which MismatchArray comes from), even if it has no use for s
ynthetic mismatch data. If these three functions are only needed in test sui
tes of other Register* packages, they should either live in a dedicated te
st-utilities package or be unexported internal helpers.

2. Counter yields Vector{Int}, not CartesianIndex

The iterate protocol returns a mutable Vector{Int} rather than Julia's s
tandard CartesianIndex. Values produced by a Counter cannot be used dire
ctly as array indices, cannot be passed to Base functions dispatching on Ca rtesianIndex, and do not compose with CartesianIndices-based code. If the
motivation was in-place mutation for performance, that's a legitimate reaso
n — but worth evaluating whether the savings justify the composability cost.

3. No compat bounds in Project.toml

Project.toml lists RegisterCore as a dependency but the [compat] secti
on is absent. The package will accept any version of RegisterCore, includi
ng future breaking releases.

Design Questions

Q1: Why does Counter exist given CartesianIndices?
CartesianIndices in Base covers Cartesian iteration, and CartesianIndex
supports arithmetic. Was the design intent that callers need a mutable, mod
ifiable
index vector (in-place writes without allocation), or was it that CartesianIndex arithmetic wasn't available at time of writing? If the forme
r, worth documenting. If the latter, Counter may be a candidate for deprec
ation.

Q2: Should the two quadratic methods have different visibility?
One returns a plain Matrix{Float64}, the other wraps the result in a Mism atchArray. If the matrix form is only meaningful as an intermediate step, w
as the intent for it to be exported?

Q3: What is tighten's intended use domain?
Described as useful for "removing unnecessary type looseness in test fixtures." If purely for tests, it arguably belongs in test/ rather than being exported. If there are production
callers, a docstring explaining the motivation would help.

Q4: Is block_center's formula intentional for even sizes?
block_center computes centers as sz[i] >> 1 + 1. For even sizes this returns the upper-center element; for odd sizes the true center. Is this asymmetry intentional (e.g., matching
a convention in RegisterCore)?

Observations

  • Counter does not implement Base.length, Base.size, Base.eltype, or Base.ndims. These are natural for a fixed-size grid iterator and would enable collect, comprehensions,
    and IteratorSize-aware code.
  • The Counter struct has a field named max, which shadows Base.max.
  • No docs/ directory and no docstrings on any exported function.
  • No public annotations for non-exported names (none identified).

Summary

The main design tension is that the package houses two conceptually distinct things — a general-purpose mutable Cartesian iterator (Counter) and domain-specific testing scaffolding
(quadratic, block_center, tighten) — without a clear organizing principle, forcing RegisterCore as a transitive dependency on any package that only needs Counter.

Highest-leverage changes:

  1. Clarify whether Counter is a deliberate alternative to CartesianIndices or a legacy construct.
  2. Move testing utilities to a separate package or unexport them if their only consumers are test/ directories downstream.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions