fix(justdummies): give the values their identity, and declare what a value is - #393
Merged
Conversation
Both are immutable, both are built through factories, and both say in their own remarks that they are values like every other here — and both answered "is this the same one?" by reference, which is the answer a reference type gives silently when nobody writes another. ConstraintCall carried its equality because code compared constraints with ==, so the gap was visible there. Nothing compares two claims or two replays today, so nothing forced the question, and it went unasked. A value that would answer wrongly the first time it is asked is worse than one that answers now. The two identities are not the text alone. A claim compares its constraint alongside its wording, because a phrase reading like WithLength(3) is not the constraint WithLength(3) — only the latter can be recognised as the one being applied, which is what the blame choice turns on. A replay compares its guidance alongside its seed, because the same seed replays a run in full or only in part. Neither comparison nor hashing composes anything, so both stay within ADR-0064: they cannot fail while a failure is being reported. The generators and the specifications are deliberately left alone. They are immutable too, but they are recipes rather than values — two identically constrained generators are two recipes, and comparing them by value would claim a meaning they do not have. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Di41ybdgmZ8tTkfvYTjGNr
A reference type compares by identity when nobody writes another answer, and it does so silently — no warning, no failing test, nothing for a reviewer to catch. That is how two of this library's three values shipped without one: only ConstraintCall had its equality, because code happened to compare constraints with == and forced the question. [ValueObject] makes the claim explicit and ValueObjectConventionTests holds it to the contract by reflection: sealed, immutable, IEquatable<T>, both Equals overloads, GetHashCode, and the ==/!= pair — the pair being the silent half, since its absence compiles and compares references where a missing Equals would at least be visible in the type. It is a declaration rather than a detection, and deliberately so. Immutability alone would sweep in the generators and the specifications, which are immutable recipes: two identically constrained generators are two recipes, not one value, and comparing them by value would claim a meaning they do not have. Only a type that says it is a value is held to the contract. Structure is what reflection can settle, and it is the half that goes missing; whether two equal instances really hash alike stays with each type's own tests. The convention was checked against a specification marked on purpose — five violations, one line each — and against a field made writable, so neither branch passes by accident. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Di41ybdgmZ8tTkfvYTjGNr
Drafted as Proposed, per the agent procedure: the decision is the maintainer's to accept. It records why value-object-hood is declared rather than detected — immutability would sweep in the generators and the specifications, which are recipes — and why the operator pair is the member most worth enforcing, being the only one whose absence changes behaviour without changing whether the code compiles. It also states what the convention deliberately does not check: whether two equal instances hash alike, and whether the fields chosen for equality are the right ones, are questions about a type's meaning that no reflection over its shape can answer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Di41ybdgmZ8tTkfvYTjGNr
A value that does not override ToString shows a debugger the one thing its reader already knows — its type name — and nothing about that looks wrong either, which is the same shape as the equality gap this convention was written for. The form was already settled here: [DebuggerDisplay] forwarding to ToString, on the values in FirstClassErrors. It was followed there by attention alone, and the three values added since did not follow it. The convention carries it now instead of a reader. Replay had no rendering at all. Its two peers did, and that is why the omission read as deliberate rather than missed: their ToString is quoted into conflict messages, so it was load-bearing and got written. Nothing appends a replay whole — AnyGenerationException takes its guidance and its seed separately — so nothing forced the question, and the debugger was left with a type name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Di41ybdgmZ8tTkfvYTjGNr
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
2 participants
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.
Summary
Two of the library's three values —
ConstraintClaimandReplay— compared by reference despite both saying in their own remarks that they are values like every other here.ConstraintCallhad its equality only because code happened to compare constraints with==, which forced the question; nothing forced it for the other two. They get their identity, and[ValueObject]plus a reflection convention keeps the gap from reopening.Type of change
Changes
ConstraintClaimandReplaya full value identity:IEquatable<T>, bothEqualsoverloads,GetHashCode, and==/!=.WithLength(3)is not the constraintWithLength(3)— only the latter can be recognised as the one being applied, which is what the blame choice inContradictsturns on. A replay compares its guidance alongside its seed, because the same seed replays a run in full or only in part.[ValueObject]andValueObjectConventionTests, which holds every marked type to sealed, immutable,IEquatable<T>, bothEqualsoverloads,GetHashCode, and the operator pair.Testing
dotnet build FirstClassErrors.sln— 0 warnings, 0 errorsdotnet test FirstClassErrors.sln— 13 test projects greenFirstClassErrors.Analyzers.UnitTests) — run as part of the solution aboveAlso run, beyond the boxes:
JustDummies.UnitTests— 663, up from 651: 12 new cases covering every new member. Each factory, bothEqualsoverloads,GetHashCode,==,!=, and each property getter is asserted directly — a getter carries no logic, so a mutation score says nothing about it.dotnet build JustDummies.UnitTests -c Release -f net472 -p:EnableNet472Floor=true— the .NET Framework support floor: 0 warnings, 0 errors.ConstraintClaim.csandReplay.cs: 100%, 30/30 killed.field '_rendered' is not readonly.Documentation
doc/updated — ADR-0066 in English and French, indexedArchitecture decisions
Proposed: ADR-0066Two things worth your eye rather than a reviewer's.
Replayhas noToString()where its two peers do, and it stays that way on purpose:AnyGenerationExceptionreadsGuidanceandSeedseparately, so nothing renders a replay whole, and adding one would be a member nothing exercises. And the convention checks structure only — whether two equal instances hash alike, and whether the fields chosen for equality are the right ones, stay with each type's own tests. ADR-0066 states both rather than leaving them to be assumed. @ReefactRelated issues
Generated by Claude Code