-
Notifications
You must be signed in to change notification settings - Fork 253
Consolidate OneOrMany implementation and its usage. #513
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Consolidates the OneOrMany<T>
API by exposing a List
property and updating all existing consumers to use it, and adds tests for the new List
accessor.
- Added
List
andSingleOrDefault
properties with XML documentation toOneOrMany<T>
. - Refactored
SearchValidator
,SearchMemoryEvaluator
, and EFSearchEvaluator
to call the newList
property instead of type-checkingValues
. - Introduced unit tests for the
List
accessor inOneOrManyTests
.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
tests/Ardalis.Specification.Tests/Internals/OneOrManyTests.cs | Added tests for the new List property |
src/Ardalis.Specification/Internals/OneOrMany.cs | Implemented List and SingleOrDefault accessors |
src/Ardalis.Specification/Validators/SearchValidator.cs | Switched from checking Values to using List directly |
src/Ardalis.Specification/Evaluators/SearchMemoryEvaluator.cs | Updated to use List accessor for search expressions |
src/Ardalis.Specification.EntityFrameworkCore/Evaluators/SearchEvaluator.cs | Updated EF evaluator to span over the new List property |
Comments suppressed due to low confidence (2)
tests/Ardalis.Specification.Tests/Internals/OneOrManyTests.cs:214
- Tests cover the
List
accessor but do not verify the newSingleOrDefault
property behavior. Add tests forSingleOrDefault
in the empty, single, and multiple item scenarios.
}
src/Ardalis.Specification/Internals/OneOrMany.cs:75
- [nitpick] Exposing the mutable
List<T>
allows external callers to modify internal state. Consider returningIReadOnlyList<T>
or a defensive copy to preserve encapsulation.
public readonly List<T> List
It addresses the issue #506