Skip to content

Spatial/unit interpretation is duplicated across the codebase and bypasses the Converter #444

Description

@GondekNP

Summary

The engine already has a unit Converter subsystem, yet almost none of the grid/spatial code uses it. Instead, "what does this unit mean" is re-decided via hardcoded string comparisons (equals("m"), contains("degree"), …) that are copy-pasted across the Java engine, the JVM/browser facade, and the JS editor. The logic is duplicated, inconsistent, drifts across the Java/JS boundary, and contradicts the documented unit behavior. It has already produced at least one real bug (the km grid-size preprocessing hang, mitigated in #442).

This issue is an inventory of the non-DRY surface so we can converge on a single source of truth. It intentionally does not propose a specific design.

We already have a Converter

org.joshsim.engine.value.converter provides everything needed to reason about units in one place:

  • Converter / MapConverter — look up a Conversion between two Units.
  • ConverterBuilder — assembles the default conversions; supports a transitive graph (extendTransitively) so registering A↔B and B↔C yields A↔C.
  • NoopConversion — pure synonyms / aliases (factor 1, e.g. m=meter=meters).
  • DirectConversion — scaling conversions (e.g. km=1000m), including user-defined ones via the start unit … end unit stanza (alias and x = current / n).
  • Units — parsing/caching, compound units (kg / m * m), simplification.

The grid/spatial layer does not consult any of this. It re-implements a tiny, lossy subset (an allow-list of meter spellings) by hand.

The non-DRY / inconsistent sites

"Is the cell size in meters?" — literal {m, meter, meters} allow-list (4 Java + 1 JS)

  • src/main/java/org/joshsim/lang/bridge/GridFromSimFactory.java:86-90
  • src/main/java/org/joshsim/command/RunUtil.java:575
  • src/main/java/org/joshsim/JoshSimFacade.java:111-113
  • src/main/java/org/joshsim/command/PreprocessUtil.java:351-352 (unitsSupported() whitelist; gate at :265)
  • editor/js/wasm.js:306-308 (_isMeters)

"Are positions in degrees?" — substring match on "degree"/"degrees" (3 Java + JS)

  • src/main/java/org/joshsim/lang/bridge/GridFromSimFactory.java:85 ("degrees")
  • src/main/java/org/joshsim/command/RunUtil.java:571 ("degree")
  • src/main/java/org/joshsim/JoshSimFacade.java:106 ("degree")
  • editor/js/wasm.js:317-319 (_isDegrees), editor/js/exporter.js:55, 267, 306 (metadata.hasDegrees())

Note the checks aren't even consistent with each other: some match "degrees", some "degree", some equals("m") only, others include meter/meters.

The spatial-mode decision (hasDegrees && sizeMeters → earth space) duplicated wholesale

  • src/main/java/org/joshsim/lang/bridge/GridFromSimFactory.java:91-98
  • src/main/java/org/joshsim/command/RunUtil.java:571-576 + :722
  • src/main/java/org/joshsim/JoshSimFacade.java:106-116
  • editor/js/wasm.js:227-246 (degrees+meters → Haversine grid sizing, reimplemented in JS)

count/empty unit special-casing outside the Converter

  • src/main/java/org/joshsim/engine/value/type/EngineValue.java:687-688 (isEmpty() || equals("count"))
  • src/main/java/org/joshsim/engine/value/converter/Units.java (cache maps count → empty string)

Hardcoded conversion constants living outside the Converter

  • src/main/java/org/joshsim/engine/geometry/HaversineUtil.java:17-19 (EARTH_RADIUS_METERS = 6371000; the de facto degrees→meters conversion) — note the spec says degrees→meters is a "default" conversion, but it lives here, not in the Converter.

Documented behavior the code doesn't honor

LanguageSpecification.md:692-693 states, as defaults:

  • Degrees (degrees, degree) for location … are uni-directionally convertable to meters by haversine distance.
  • Meters (alias: meter, m) and kilometers (alias: kilometer, km) are defined by default and mutually convertable.

Neither is registered in ConverterBuilder (only m↔meter↔meters, count↔counts, ms↔millisecond↔milliseconds). Consequences:

  • grid.size = … km is taken at face value and treated as meters, producing a grid ~1000× too fine → preprocessing hang. Fail fast on unsupported grid.size units in preprocess #442 converts that into a fast, clear error, but km still isn't accepted, even though the docs promise it.
  • The degrees→meters conversion exists only inside the grid code (Haversine), not as a Conversion users or other code can reach.

The legitimate constraint (for context)

This is genuinely not "just convert everything to meters": m under a projected/UTM CRS means already metric, no Haversine, while m with degree positions means needs Haversine. So position-units (and CRS) determine the spatial mode, independent of the size magnitude. That distinction is real — but today it's re-derived independently at every site above rather than decided once.

Why this matters

  • Drift: the Java engine and JS editor maintain parallel, subtly different copies of the same rules.
  • Correctness: documented units (km, degrees→m) silently don't work.
  • Extensibility: adding a unit (cm, mi, projected CRSs, time units) means editing N hardcoded sites instead of registering one Conversion.

Related

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions