fix: replace Price object in TransactionWasCreated with primitives#46
Merged
jorge07 merged 2 commits intojorge07:masterfrom Feb 22, 2026
Merged
Conversation
Domain events should carry only primitive data so they are safe to serialize and replay without coupling the event schema to domain classes. TransactionWasCreated previously embedded a Price value object. If Price gains or changes a field, old events deserialized from the event store would be silently inconsistent — a latent replay correctness bug. Fix: TransactionWasCreated now carries amount: number and currency: string. Transaction.onTransactionWasCreated() reconstructs Price from these primitives. Transactions.fromCreated() uses event.amount and event.currency directly. Transaction.test.ts updated: asserts event.amount and event.currency instead of event.price (which no longer exists on the event). Source: Young — "Events are the durable record. They must be self-describing with primitive data." CQRS Documents (2010).
jorge07
reviewed
Feb 22, 2026
Owner
jorge07
left a comment
There was a problem hiding this comment.
this needs precision, not the best way to deal with money in the long run
IEEE 754 floats cannot represent all decimal values exactly. Storing money amounts as float is a correctness risk — 0.1 + 0.2 yields 0.30000000000000004, which serialised into an event store produces a permanently incorrect durable record. Fix: Price.amount is now a string (e.g. "12.50"). Validators convert to Number() only for range checks; the raw string is stored unchanged. Propagation: - TransactionWasCreated.amount: number → string (event carries string) - Transaction.onTransactionWasCreated: new Price(event.amount, event.currency) reconstructs directly — no float conversion - CreateCommand: String(price.amount) converts the HTTP JSON number to string at the application boundary before constructing Price - Transactions.fromCreated: Number(event.amount) for the float DB column (acceptable in the read model; event store remains precise) All callers updated. Tests updated to pass string literals to Price.
Contributor
Author
|
Good call. Addressed in a second commit: What changed:
For a production system the next step would be accepting amount as string (or integer minor units) from the HTTP API too — but that's a breaking API change and Phase 4 territory. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
TransactionWasCreatedwas carrying aPricevalue object instead of primitive data. This couples the event schema to thePriceclass — a latent replay correctness bug.Why it matters
Domain events are the durable record of what happened. They are serialized to the event store and replayed to rebuild aggregates. If
Priceever changes (e.g., gains ataxRatefield), old events deserialized from storage will be structurally inconsistent with the new class — silently producing wrong aggregate state on replay.Before
After
Transaction.onTransactionWasCreated()reconstructsPricefrom primitives:Transactions.fromCreated()readsevent.amount/event.currencydirectly (no change in semantics, removes property traversal through a domain class).Transaction.test.tsupdated to assertevent.amountandevent.currencyindividually.Verification