refactor: add Into derives to nutype types with domain operations #334

2026-04-11T08:07:09-07:00

jwilger-ai-bot commented

2026-04-11 08:07:09 -07:00

(Migrated from github.com)

Summary

Add Into, PartialOrd, and Ord derives to nutype domain types (StreamId, StreamPrefix, StreamVersion, StreamPosition, BatchSize)
Encapsulate test MoneyAmount arithmetic in domain operations:
- AccountBalance gains deposit(), withdraw(), has_sufficient_funds(), balance_cents() methods
- From<MoneyAmount> for i32 for fold-based balance computations
- Collections use Vec<MoneyAmount> with sort() instead of Vec<u16> with raw extraction
- Assertions compare against test_amount(N) instead of raw literals
Production code retains into_inner() at IO boundaries (SQL params, logging) — unchanged

Test plan

All 198 workspace tests pass
Clippy clean
No inner value extraction in domain logic or test assertions
Guardrails PR #340 merged codifying the domain-type-operations rule

Closes #245

## Summary - Add `Into`, `PartialOrd`, and `Ord` derives to nutype domain types (`StreamId`, `StreamPrefix`, `StreamVersion`, `StreamPosition`, `BatchSize`) - Encapsulate test `MoneyAmount` arithmetic in domain operations: - `AccountBalance` gains `deposit()`, `withdraw()`, `has_sufficient_funds()`, `balance_cents()` methods - `From<MoneyAmount> for i32` for fold-based balance computations - Collections use `Vec<MoneyAmount>` with `sort()` instead of `Vec<u16>` with raw extraction - Assertions compare against `test_amount(N)` instead of raw literals - Production code retains `into_inner()` at IO boundaries (SQL params, logging) — unchanged ## Test plan - [x] All 198 workspace tests pass - [x] Clippy clean - [x] No inner value extraction in domain logic or test assertions - [x] Guardrails PR #340 merged codifying the domain-type-operations rule Closes #245

jwilger (Migrated from github.com) reviewed

2026-04-11 09:14:57 -07:00

jwilger (Migrated from github.com) left a comment

No description provided.

There are a lot of places (more than I individually pointed out) where we could make the code cleaner by implementing `From`. That said, I also find myself wondering if an even better answer would be to have named domain functions that perform the operations where we are needing to do all of this casting so that any necessary casting can be encapsulated in those functions (e.g. adding or subtracting `MoneyAmount` instances, etc.

jwilger-ai-bot (Migrated from github.com) reviewed

2026-04-11 09:17:43 -07:00

eventcore-examples/tests/multi_stream_atomic_test.rs

Agreed — the { let v: u16 = (*amount).into(); v } pattern is ugly. This happened because nutype's Into derive generates Into<inner_type> but Rust can't infer the target type in these contexts, forcing the explicit binding.

Looking at the overall review comment too: for test-only types like MoneyAmount, I think From<MoneyAmount> for i32 is the right fix. Domain functions (add/subtract) would be overkill here since MoneyAmount only exists in test files.

For the production nutype types (StreamVersion, BatchSize, etc.) that have the same ugly pattern in the backends, I'll revert those back to into_inner() — it was actually cleaner before. The Into derive is useful for contexts where .into() type inference works naturally, but not for these explicit-cast-needed spots.

Fixing in the next push.

Agreed — the `{ let v: u16 = (*amount).into(); v }` pattern is ugly. This happened because nutype's `Into` derive generates `Into<inner_type>` but Rust can't infer the target type in these contexts, forcing the explicit binding. Looking at the overall review comment too: for test-only types like `MoneyAmount`, I think `From<MoneyAmount> for i32` is the right fix. Domain functions (add/subtract) would be overkill here since `MoneyAmount` only exists in test files. For the production nutype types (`StreamVersion`, `BatchSize`, etc.) that have the same ugly pattern in the backends, I'll revert those back to `into_inner()` — it was actually cleaner before. The `Into` derive is useful for contexts where `.into()` type inference works naturally, but not for these explicit-cast-needed spots. Fixing in the next push.

jwilger-ai-bot (Migrated from github.com) reviewed

2026-04-11 09:17:45 -07:00

eventcore-examples/tests/multi_stream_atomic_test.rs

Will fix with From<MoneyAmount> for u16 — then these become just (*amount).into() or amount.into().

Will fix with `From<MoneyAmount> for u16` — then these become just `(*amount).into()` or `amount.into()`.

jwilger-ai-bot (Migrated from github.com) reviewed

2026-04-11 09:17:46 -07:00

eventcore-examples/tests/multi_stream_atomic_test.rs

					
				@ -350,24 +368,24 @@ async fn concurrent_transfers_produce_consistent_final_state() {

				        .collect();

Same fix — From<MoneyAmount> for u16.

Same fix — `From<MoneyAmount> for u16`.

jwilger-ai-bot (Migrated from github.com) reviewed

2026-04-11 09:55:03 -07:00

eventcore-examples/tests/multi_stream_atomic_test.rs

Fixed. The approach now:

AccountBalance has domain methods: deposit(), withdraw(), has_sufficient_funds(), balance_cents()
From<MoneyAmount> for i32 for fold-based balance computations
Collections use Vec<MoneyAmount> with sort() (derived Ord) — no raw value extraction
Assertions compare against test_amount(N) instead of raw literals
Withdraw::handle() uses state.has_sufficient_funds(self.amount) instead of extracting inner values

Also created guardrails PR #340 (merged) codifying the domain-type-operations rule so this pattern is enforced going forward.

Fixed. The approach now: - `AccountBalance` has domain methods: `deposit()`, `withdraw()`, `has_sufficient_funds()`, `balance_cents()` - `From<MoneyAmount> for i32` for fold-based balance computations - Collections use `Vec<MoneyAmount>` with `sort()` (derived `Ord`) — no raw value extraction - Assertions compare against `test_amount(N)` instead of raw literals - `Withdraw::handle()` uses `state.has_sufficient_funds(self.amount)` instead of extracting inner values Also created guardrails PR #340 (merged) codifying the domain-type-operations rule so this pattern is enforced going forward.

jwilger-ai-bot (Migrated from github.com) reviewed

2026-04-11 09:55:04 -07:00

eventcore-examples/tests/multi_stream_atomic_test.rs

Fixed — collections now hold Vec<MoneyAmount> directly, no extraction needed.

Fixed — collections now hold `Vec<MoneyAmount>` directly, no extraction needed.

jwilger-ai-bot (Migrated from github.com) reviewed

2026-04-11 09:55:05 -07:00

eventcore-examples/tests/multi_stream_atomic_test.rs

					
				@ -350,24 +368,24 @@ async fn concurrent_transfers_produce_consistent_final_state() {

				        .collect();

Fixed — same approach, Some(*amount) keeps the domain type.

Fixed — same approach, `Some(*amount)` keeps the domain type.

jwilger (Migrated from github.com) approved these changes

2026-04-11 10:01:29 -07:00

Sign in to join this conversation.

No reviewers

jwilger

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

jwilger/eventcore!334

				`@ -350,24 +368,24 @@ async fn concurrent_transfers_produce_consistent_final_state() {`
				`.collect();`

Rows
Columns