Add conventions: local functions, tests vs production code, EF navigations#4
Merged
Conversation
Extract a normal method instead; a trivial helper of up to ~3 lines is the only exception. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Three written conventions (wording taken from the global rules), all docs-only — the codebase already complies with each. Each lives in CLAUDE.md and docs/technical-requirements.md.
No nested local functions — write a normal method
Don''t reach for local functions; extract a private method instead. A one-liner local function can occasionally be acceptable, but that''s an edge case, not something to aim for. No Roslyn analyzer covers this, so it lives as a written convention.
Verified: swept all C# for local-function declaration shapes (modifier-prefixed, built-in/custom/tuple return types, expression-bodied, top-level-statement files) — zero hits.
Tests never reshape production code
We use DDD: the domain model is designed for the domain, not for testability. If a test needs some capability, the test figures out how to achieve it — never add methods, parameters, setters, or hooks to production code just so a test can reach something. Dependency injection should be enough; otherwise the test manipulates state itself (seeding through EF, reflection as a last resort).
Verified: all entity properties are
private set, noInternalsVisibleToanywhere, tests seed exclusively through domain entities + EF. (public partial class Program;stays — the standardWebApplicationFactoryhost idiom, not a domain signature.)Entity Framework: set the navigation property together with the FK
When assigning a relationship by ID (e.g.
ProductId), also assign the navigation property (Product) — in constructors, factory methods, and setters alike — so that right after the call,something.Productis usable and non-null. The exception is performance-sensitive work like inserting thousands of rows in a batch: there it''s fine to set only the ID rather than load entities into memory that nothing will read.Verified: the current model has no reference navigations at all (
Product.ReviewsandReview.Votesare collection navigations on the principal side; dependents carry only FKs), so no call site can violate the rule today — it binds the moment a reference navigation is introduced.🤖 Generated with Claude Code