-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Context
In SourceGenerator.Emitter.cs, each Parse method has a corresponding TryParse method. The two variants differ only in error handling strategy:
- Parse throws
FormatExceptionon invalid input and returns the constructed value directly (return new T(...)) - TryParse returns
falseon invalid input and assigns the result via anoutparameter (result = new T(...); return true)
The existing shared static helpers already handle both paths via a bool shouldThrow parameter:
WriteLengthCheck(SourceWriter writer, List<KeyPart> parts, string inputName, bool shouldThrow)(line 337) — emits eitherthrow new FormatException(...)orreturn falsebased onshouldThrowWriteSplitImplementation(SourceWriter writer, List<KeyPart> parts, string inputName, out string partRangesVariableName, bool shouldThrow, out string? partCountVariableName)(line 349) — same pattern for split validationWriteParsePropertiesImplementation(...)(line 432) — same pattern for per-property parsing
What remains duplicated is the orchestration code — the local functions that call these shared helpers and wrap them with method signatures, null checks, and return statements.
In EmitForPrimaryKey (lines 48-135)
Two local functions duplicate ~40 lines of orchestration:
WriteParseMethodImplementation()(line 61) — writes theParse(string)andParse(ReadOnlySpan<char>)overloads, callsWriteLengthCheck,WriteSplitImplementation,WriteParsePropertiesImplementationwithshouldThrow: true, ends withreturn new T(...)WriteTryParseMethodImplementation()(line 94) — writes theTryParse(string?, out T?)andTryParse(ReadOnlySpan<char>, out T?)overloads, calls the same helpers withshouldThrow: false, ends withresult = new T(...); return true
The method bodies between lines 74-91 and 113-133 are structurally identical — only the method signatures, null-handling preamble, shouldThrow value, and return statement differ.
In EmitForCompositePrimaryKey (lines 137-315)
Four local functions duplicate ~50 lines of orchestration across two pairs:
Pair 1 — full primary key parse (split into partition + sort, then delegate):
WriteParseMethodImplementation()(line 166) — writesParse(string)/Parse(ReadOnlySpan<char>), callsWritePrimaryKeySplit(true)WriteTryParseMethodImplementation()(line 189) — writesTryParse(string?, out T?)/TryParse(ReadOnlySpan<char>, out T?), callsWritePrimaryKeySplit(false)
Pair 2 — composite parse (partition key + sort key separately):
WriteCompositeParseMethodImplementation()(line 218) — writesParse(string, string)/Parse(ReadOnlySpan<char>, ReadOnlySpan<char>), calls shared helpers withshouldThrow: trueWriteCompositeTryParseMethodImplementation()(line 263) — writesTryParse(string, string, out T?)/TryParse(ReadOnlySpan<char>, ReadOnlySpan<char>, out T?), calls same helpers withshouldThrow: false
Existing precedent
WritePrimaryKeySplit(bool shouldThrow) (line 155) is already a local function inside EmitForCompositePrimaryKey that follows exactly this pattern — a single local function parameterised by shouldThrow that emits either throw new FormatException(...) or return false. This refactoring applies the same approach to the broader orchestration local functions.
Blocked by
Tasks
- In
EmitForPrimaryKey: Collapse the two local functionsWriteParseMethodImplementation()andWriteTryParseMethodImplementation()into a single local function parameterised byshouldThrow(following theWritePrimaryKeySplit(bool shouldThrow)precedent). The caller provides the method signature wrapper; the shared body callsWriteLengthCheck,WriteSplitImplementation,WriteParsePropertiesImplementation, and writes the appropriate return statement based onshouldThrow. - In
EmitForCompositePrimaryKey: Same extraction forWriteCompositeParseMethodImplementation()andWriteCompositeTryParseMethodImplementation()— collapse into a single local function parameterised byshouldThrow. The first pair (WriteParseMethodImplementation/WriteTryParseMethodImplementation) can likely be unified similarly. - Keep
EmitForPrimaryKeyandEmitForCompositePrimaryKeyas separate methods — they have genuinely different method signatures (2 vs 4 Parse overloads, sort key methods) and should not be merged. - Verify snapshot tests pass — run
dotnet test src/CompositeKey.SourceGeneration.UnitTests -c Release. - Run functional tests — run
dotnet test src/CompositeKey.SourceGeneration.FunctionalTests -c Release. - Run benchmarks to confirm no performance regression.
Notes
- All functions being unified are local functions within their respective
EmitFor*methods, not top-level or static helper methods on the class. The refactored versions should remain local functions. - The generated output (source code emitted by the generator) must not change — only the generator's internal structure is being refactored. Snapshot tests will verify this.
- The shared static helpers (
WriteLengthCheck,WriteSplitImplementation,WriteParsePropertiesImplementation) are not changed by this refactoring — they already acceptshouldThrowand work correctly for both paths. This step is purely about the ~90 lines of orchestration code that wraps calls to those helpers.