feat: Added additional Methods, for better support #523
Conversation
samtrion
added
state:ready for merge
WalkthroughAdds many new argument-validation polyfills (strings, collections, counts, duplicates, defaults, GUIDs, generic ranges, and NET8+ date/time helpers), expands unit tests across these behaviors, and replaces/extends README documentation and project branding to "Arguments". Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/NetEvolve.Arguments/ArgumentExceptionPolyfill.cs`:
- Around line 260-278: The IEnumerable overloads (e.g.,
ThrowIfCountGreaterThan<T>(IEnumerable<T>? argument, int maxCount, ...))
currently skip validation when TryGetNonEnumeratedCount returns false; change
these methods so that after the null check they attempt TryGetNonEnumeratedCount
and if it returns false fall back to enumerating the sequence to compute the
actual count (stop early once count > maxCount for efficiency) and then throw
the existing ArgumentException when the computed count violates the constraint;
apply the same fallback enumeration change to the other two IEnumerable-based
count validators that use TryGetNonEnumeratedCount so non-countable iterators
are validated consistently.
In `@src/NetEvolve.Arguments/README.md`:
- Around line 7-12: The README currently lists two different minimum .NET
Framework versions (4.6.2 in the intro and 4.7.2 in the Features/Universal
Compatibility section); update the intro text to use the same version as the
Features section (change "4.6.2+" to "4.7.2+") so the minimum framework
requirement is consistent across the file, and scan other occurrences of
"4.6.2+" (e.g., near the "ArgumentNullException.ThrowIf*" mention and the
"Universal Compatibility" bullet) to replace them with "4.7.2+".
In
`@tests/NetEvolve.Arguments.Tests.Unit/ArgumentOutOfRangeException_ThrowIfInPastTests.cs`:
- Around line 21-30: The test method name
ArgumentOutOfRangeException_ThrowIfInPastTests.ThrowIfInPast_DateTimeOffset_WhenValueIsNow_DoesNotThrow
is misleading because it creates a value with
DateTimeOffset.UtcNow.AddSeconds(1) (a future value); either rename the test to
reflect that (e.g.,
ThrowIfInPast_DateTimeOffset_WhenValueIsSlightlyInFuture_DoesNotThrow) or change
the arranged value to DateTimeOffset.UtcNow (or another explicit boundary value)
so the name and behavior match; update the test method identifier and any
references accordingly and keep the assertion using
ArgumentOutOfRangeException.ThrowIfInPast and the subsequent Assert unchanged.
🧹 Nitpick comments (3)
tests/NetEvolve.Arguments.Tests.Unit/ArgumentException_ThrowIfCountGreaterThanTests.cs (1)
56-93: Consider adding null tests for ICollection, IReadOnlyCollection, and Array overloads.The IEnumerable tests include a null-input test (lines 8-19), but the ICollection, IReadOnlyCollection, and Array sections only test the "exceeds maximum" scenario. For consistency with
ArgumentException_ThrowIfCountLessThanTests.cs(which tests null for all collection types), consider adding:
ThrowIfCountGreaterThan_ICollection_WhenNull_ThrowsArgumentNullExceptionThrowIfCountGreaterThan_IReadOnlyCollection_WhenNull_ThrowsArgumentNullExceptionThrowIfCountGreaterThan_Array_WhenNull_ThrowsArgumentNullExceptiontests/NetEvolve.Arguments.Tests.Unit/ArgumentOutOfRangeException_ThrowIfInPastTests.cs (1)
43-65: Consider adding a "now" boundary test for DateTime.The DateTimeOffset section includes a "WhenValueIsNow" test (lines 21-30), but the DateTime section only tests past and future cases. For consistency, consider adding a
ThrowIfInPast_DateTime_WhenValueIsNow_DoesNotThrowtest.tests/NetEvolve.Arguments.Tests.Unit/ArgumentException_ThrowIfCountLessThanTests.cs (1)
56-126: Consider adding "less than minimum" throwing tests for ICollection, IReadOnlyCollection, and Array.The IEnumerable section thoroughly tests all scenarios (null, less than, equals, greater than), but the ICollection, IReadOnlyCollection, and Array sections only test null and "greater than minimum" cases. For consistency, consider adding tests like:
ThrowIfCountLessThan_ICollection_WhenCountLessThanMinimum_ThrowsArgumentExceptionThrowIfCountLessThan_IReadOnlyCollection_WhenCountLessThanMinimum_ThrowsArgumentExceptionThrowIfCountLessThan_Array_WhenCountLessThanMinimum_ThrowsArgumentExceptionThis would ensure each overload's throwing behavior is explicitly verified.
tests/NetEvolve.Arguments.Tests.Unit/ArgumentOutOfRangeException_ThrowIfInPastTests.cs
Show resolved
Hide resolved
samtrion
force-pushed
the
feature/extended-methods
branch
from
4db1408 to
25afa49
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/NetEvolve.Arguments/ArgumentExceptionPolyfill.cs (1)
64-84: IEnumerableThrowIfNullOrEmptyskips empty iterators when count isn’t available.If
TryGetNonEnumeratedCountreturns false, empty iterators pass without throwing, which violates the method’s contract. Consider a lightweight fallback that checksMoveNext()before returning.🛠️ Proposed fix (early enumeration fallback)
- if (argument.TryGetNonEnumeratedCount(out var count) && count == 0) - { - throw new ArgumentException("The collection cannot be empty.", paramName); - } + if (argument.TryGetNonEnumeratedCount(out var count)) + { + if (count == 0) + { + throw new ArgumentException("The collection cannot be empty.", paramName); + } + } + else + { + using var enumerator = argument.GetEnumerator(); + if (!enumerator.MoveNext()) + { + throw new ArgumentException("The collection cannot be empty.", paramName); + } + }
🤖 Fix all issues with AI agents
In `@README.md`:
- Line 226: Replace the bare email address in the README sentence that currently
reads "Please report unacceptable behavior to info@daily-devops.net." with a
mailto link (e.g., <mailto:info@daily-devops.net>) so the line becomes a
clickable mailto anchor and satisfies markdownlint rule MD034; simply update the
Markdown content in README.md to use the mailto-style link.
In `@src/NetEvolve.Arguments/README.md`:
- Around line 491-493: Update the README heading "Backward Compatible Argument
Class (Obsolete)" to hyphenate the compound adjective so it reads
"Backward-Compatible Argument Class (Obsolete)"; locate the heading text in the
README.md and change only the heading text, leaving the `Argument` class name
and surrounding content untouched.
🧹 Nitpick comments (1)
tests/NetEvolve.Arguments.Tests.Unit/ArgumentException_ThrowIfCountLessThanTests.cs (1)
69-126: Add boundary tests for ICollection/IReadOnlyCollection/array overloads.To match the IEnumerable coverage, consider adding “count < min” and “count == min” tests for the other overloads too.
🧪 Example additions (ICollection)
+ [Test] + public void ThrowIfCountLessThan_ICollection_WhenCountLessThanMinimum_ThrowsArgumentException() + { + // Arrange + ICollection<int> argument = [1, 2]; + + // Act + void Act() => ArgumentException.ThrowIfCountLessThan(argument, 5); + + // Assert + _ = Assert.Throws<ArgumentException>("argument", Act); + } + + [Test] + public async Task ThrowIfCountLessThan_ICollection_WhenCountEqualsMinimum_DoesNotThrow() + { + // Arrange + ICollection<int> argument = [1, 2, 3, 4, 5]; + + // Act & Assert + ArgumentException.ThrowIfCountLessThan(argument, 5); + _ = await Assert.That(argument.Count).IsEqualTo(5); + }
Simplified collection count and duplicate checks in ArgumentExceptionPolyfills. Improved default value comparison. Added comprehensive unit tests for null and empty collections across all overloads. Updated test for Guid default value. Suppressed IDE1006 warnings in test project.
…pdate README for .NET Framework support
samtrion
force-pushed
the
feature/extended-methods
branch
from
caf35a6 to
f4f5c4b
Compare
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (68.75%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #523 +/- ##
============================================
- Coverage 100.00% 77.94% -22.06%
============================================
Files 13 13
Lines 100 340 +240
Branches 26 92 +66
============================================
+ Hits 100 265 +165
- Misses 0 57 +57
- Partials 0 18 +18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/NetEvolve.Arguments/ArgumentExceptionPolyfill.cs (1)
64-84: Ensure IEnumerable emptiness is validated when count is unavailable.If
TryGetNonEnumeratedCountreturns false, empty iterators currently pass validation silently, which breaks the method contract.✅ Proposed fix
- if (argument.TryGetNonEnumeratedCount(out var count) && count == 0) - { - throw new ArgumentException("The collection cannot be empty.", paramName); - } + if (argument.TryGetNonEnumeratedCount(out var count)) + { + if (count == 0) + { + throw new ArgumentException("The collection cannot be empty.", paramName); + } + } + else + { + using var enumerator = argument.GetEnumerator(); + if (!enumerator.MoveNext()) + { + throw new ArgumentException("The collection cannot be empty.", paramName); + } + }
🧹 Nitpick comments (3)
src/NetEvolve.Arguments/ArgumentExceptionPolyfill.cs (1)
379-423: Short‑circuit non‑countable enumerables to avoid full/infinite enumeration.The fallback branches fully enumerate sequences even when the threshold is satisfied; this can be costly or hang on unbounded sequences.
♻️ Suggested refactor
else { - var actualCount = 0; - foreach (var _ in argument) - { - actualCount++; - } - - if (actualCount < minCount) - { - throw new ArgumentException( - $"The collection count {actualCount} is less than the minimum required count {minCount}.", - paramName - ); - } + if (minCount <= 0) + { + return; + } + + var actualCount = 0; + foreach (var _ in argument) + { + if (++actualCount >= minCount) + { + return; + } + } + + throw new ArgumentException( + $"The collection count {actualCount} is less than the minimum required count {minCount}.", + paramName + ); }else { var actualCount = 0; foreach (var _ in argument) { - actualCount++; + actualCount++; + if (actualCount > maxCount) + { + throw new ArgumentException( + $"The collection count {actualCount} is outside the allowed range [{minCount}, {maxCount}].", + paramName + ); + } } - if (actualCount < minCount || actualCount > maxCount) + if (actualCount < minCount) { throw new ArgumentException( $"The collection count {actualCount} is outside the allowed range [{minCount}, {maxCount}].", paramName ); } }Also applies to: 506-552
tests/NetEvolve.Arguments.Tests.Unit/ArgumentOutOfRangeException_ThrowIfInPastTests.cs (1)
21-30: Capture a single “now” per test to reduce timing flakiness.Multiple
UtcNowcalls can drift; caching the baseline improves stability.✍️ Example tweak
public async Task ThrowIfInPast_DateTimeOffset_WhenValueIsSlightlyInFuture_DoesNotThrow() { // Arrange - var argument = DateTimeOffset.UtcNow.AddSeconds(1); + var now = DateTimeOffset.UtcNow; + var argument = now.AddSeconds(1); // Act & Assert ArgumentOutOfRangeException.ThrowIfInPast(argument); - _ = await Assert.That(argument).IsGreaterThan(DateTimeOffset.UtcNow.AddSeconds(-1)); + _ = await Assert.That(argument).IsGreaterThan(now.AddSeconds(-1)); }Apply the same pattern to the other
UtcNow/UtcNow.Add*assertions in this file.tests/NetEvolve.Arguments.Tests.Unit/ArgumentExceptionPolyfillsTests.cs (1)
95-107: Add iterator-based IEnumerable test to cover non-countable sequences.This ensures
ThrowIfNullOrEmpty(IEnumerable<T>)validates empty iterators, not just countable collections.🧪 Suggested test
public void ThrowIfNullOrEmpty_Enumerable_WhenArgumentIsNull_ThrowsArgumentNullException() { // Arrange IEnumerable<int>? argument = null; @@ _ = Assert.Throws<ArgumentNullException>("argument", Act); } + + [Test] + public void ThrowIfNullOrEmpty_Enumerable_WhenArgumentIsEmptyIterator_ThrowsArgumentException() + { + // Arrange + IEnumerable<int> argument = Empty(); + + // Act + void Act() => ArgumentException.ThrowIfNullOrEmpty(argument); + + // Assert + _ = Assert.Throws<ArgumentException>("argument", Act); + + static IEnumerable<int> Empty() + { + yield break; + } + }
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.