From a16e2cbf4d74b9702664e34b20a9f6b207acc5b2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 13:49:03 +0000 Subject: [PATCH] refactor(binder): extract shared list-element iteration The three list converters (simple, simple-value, complex) each carried an identical element loop: indexed iteration, a null element recorded as REQUEST_ARGUMENT_REQUIRED, collect every successful element, and return a RequiredField. Factor that shared invariant into a single RequestBinder.ConvertEachElement primitive so the iteration and the null-element rule live in one place; each converter now supplies only its per-element convert/bind and failure recording through a callback. No behavior change; the public binder surface is unchanged. Refs: #150 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01J12ghrzM1xiAKw4LETJDqr --- .../ListOfComplexPropertiesConverter.cs | 24 ++--------- .../ListOfSimplePropertiesConverter.cs | 28 ++----------- .../ListOfSimpleValuePropertiesConverter.cs | 30 +++----------- .../RequestBinder.cs | 40 +++++++++++++++++++ 4 files changed, 52 insertions(+), 70 deletions(-) diff --git a/FirstClassErrors.RequestBinder/ListOfComplexPropertiesConverter.cs b/FirstClassErrors.RequestBinder/ListOfComplexPropertiesConverter.cs index b979c46d..e70e026d 100644 --- a/FirstClassErrors.RequestBinder/ListOfComplexPropertiesConverter.cs +++ b/FirstClassErrors.RequestBinder/ListOfComplexPropertiesConverter.cs @@ -78,33 +78,15 @@ public RequiredField> AsOptional(Func> BindElements(Func, Outcome> bindElement) where TProperty : notnull { - List bound = new(); - int index = 0; - - foreach (TArgument? element in _values!) { - string elementPath = $"{_argumentPath}[{index}]"; - index++; - - if (element is null) { - _binder.RecordArgumentRequired(elementPath); - - continue; - } - + return _binder.ConvertEachElement(_argumentPath, _values!, (element, elementPath) => { RequestBinder nested = new(element!, _envelope, _binder.Options, elementPath); Outcome outcome = bindElement(nested); if (outcome.IsFailure) { _binder.Record(NestedFailure.Group(outcome.Error!, nested.BuiltEnvelope, elementPath, _binder.Options.ArgumentInvalidCode)); - - continue; } - bound.Add(outcome.GetResultOrThrow()); - } - - // The value is read only through a BindingScope, which a build terminal creates solely when no failure was recorded — - // i.e. only when every element bound — so `bound` is the complete list exactly when it is observed. - return new RequiredField>(_binder, bound); + return outcome; + }); } } diff --git a/FirstClassErrors.RequestBinder/ListOfSimplePropertiesConverter.cs b/FirstClassErrors.RequestBinder/ListOfSimplePropertiesConverter.cs index 031b4857..9fa9bf5c 100644 --- a/FirstClassErrors.RequestBinder/ListOfSimplePropertiesConverter.cs +++ b/FirstClassErrors.RequestBinder/ListOfSimplePropertiesConverter.cs @@ -72,32 +72,12 @@ public RequiredField> AsOptional(Func> ConvertElements(Func> convertElement) where TProperty : notnull { - List converted = new(); - int index = 0; - - foreach (TArgument? element in _values!) { - string elementPath = $"{_argumentPath}[{index}]"; - index++; - - if (element is null) { - _binder.RecordArgumentRequired(elementPath); - - continue; - } - + return _binder.ConvertEachElement(_argumentPath, _values!, (element, elementPath) => { Outcome outcome = convertElement(element!); - if (outcome.IsFailure) { - _binder.RecordArgumentInvalid(elementPath, outcome.Error!); - - continue; - } - - converted.Add(outcome.GetResultOrThrow()); - } + if (outcome.IsFailure) { _binder.RecordArgumentInvalid(elementPath, outcome.Error!); } - // The value is read only through a BindingScope, which a build terminal creates solely when no failure was recorded — - // i.e. only when every element converted — so `converted` is the complete list exactly when it is observed. - return new RequiredField>(_binder, converted); + return outcome; + }); } } diff --git a/FirstClassErrors.RequestBinder/ListOfSimpleValuePropertiesConverter.cs b/FirstClassErrors.RequestBinder/ListOfSimpleValuePropertiesConverter.cs index 3f6e1c4b..41bb2fcd 100644 --- a/FirstClassErrors.RequestBinder/ListOfSimpleValuePropertiesConverter.cs +++ b/FirstClassErrors.RequestBinder/ListOfSimpleValuePropertiesConverter.cs @@ -79,32 +79,12 @@ public RequiredField> AsOptional(Func> ConvertElements(Func> convertElement) where TProperty : notnull { - List converted = new(); - int index = 0; + return _binder.ConvertEachElement(_argumentPath, _values!, (element, elementPath) => { + Outcome outcome = convertElement(element!.Value); + if (outcome.IsFailure) { _binder.RecordArgumentInvalid(elementPath, outcome.Error!); } - foreach (TArgument? element in _values!) { - string elementPath = $"{_argumentPath}[{index}]"; - index++; - - if (element is null) { - _binder.RecordArgumentRequired(elementPath); - - continue; - } - - Outcome outcome = convertElement(element.Value); - if (outcome.IsFailure) { - _binder.RecordArgumentInvalid(elementPath, outcome.Error!); - - continue; - } - - converted.Add(outcome.GetResultOrThrow()); - } - - // The value is read only through a BindingScope, which a build terminal creates solely when no failure was recorded — - // i.e. only when every element converted — so `converted` is the complete list exactly when it is observed. - return new RequiredField>(_binder, converted); + return outcome; + }); } } diff --git a/FirstClassErrors.RequestBinder/RequestBinder.cs b/FirstClassErrors.RequestBinder/RequestBinder.cs index 087ba298..a8104f69 100644 --- a/FirstClassErrors.RequestBinder/RequestBinder.cs +++ b/FirstClassErrors.RequestBinder/RequestBinder.cs @@ -242,6 +242,46 @@ internal void RecordArgumentInvalid(string argumentPath, Error cause) { Record(RequestBindingError.ArgumentInvalid(Options.ArgumentInvalidCode, argumentPath, cause)); } + /// + /// Binds every element of a list property at its indexed path (Tags[2]), collecting every + /// failure so one bad element never hides the others: a null element records + /// REQUEST_ARGUMENT_REQUIRED, and binds each non-null element — + /// recording its own failure (a converter error, or a nested envelope wrapped under the path) and yielding + /// its value on success. The list converters share this single iteration and null-element rule, so a change + /// to either is made in one place. + /// + /// The element type as stored in the DTO list (a reference or a ). + /// The type each element binds to. + /// The list's argument path; each element is reported under argumentPath[index]. + /// The list elements. + /// Binds a non-null element at its indexed path, recording its own failure. + /// The bound field token carrying the successfully bound elements. + internal RequiredField> ConvertEachElement( + string argumentPath, IEnumerable values, Func> convertElementAt) where TProperty : notnull { + List converted = new(); + int index = 0; + + foreach (TStored element in values) { + string elementPath = $"{argumentPath}[{index}]"; + index++; + + if (element is null) { + RecordArgumentRequired(elementPath); + + continue; + } + + Outcome outcome = convertElementAt(element, elementPath); + if (outcome.IsSuccess) { converted.Add(outcome.GetResultOrThrow()); } + // A failing element was already recorded by convertElementAt (REQUEST_ARGUMENT_INVALID, or the nested + // envelope wrapped under the indexed path), so it is simply skipped here. + } + + // The value is read only through a BindingScope, which a build terminal creates solely when no failure was + // recorded — i.e. only when every element bound — so `converted` is the complete list when it is observed. + return new RequiredField>(this, converted); + } + /// Prepends this binder's argument prefix to a path segment ("CheckIn" -> "Stay.CheckIn"). internal string PathOf(string argumentName) { return _argumentPrefix is null ? argumentName : $"{_argumentPrefix}.{argumentName}";