Conversation
Agent-Logs-Url: https://github.com/Tyrrrz/PowerKit/sessions/4dc30744-0f1d-4c83-af52-2231f02919ee Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
There was a problem hiding this comment.
- Split each extension block into its own class/file.
- Add overloads that allow passing custom formatting options, etc. Make the current overloads call those.
- Test only the outermost overloads.
There was a problem hiding this comment.
Done in 8583988:
ParsingExtensions.csis deleted — each type now has its own file (IntParsingExtensions.cs,LongParsingExtensions.cs,DoubleParsingExtensions.cs,DecimalParsingExtensions.cs,BoolParsingExtensions.cs,DateTimeParsingExtensions.cs,DateTimeOffsetParsingExtensions.cs,TimeSpanParsingExtensions.cs,GuidParsingExtensions.cs).- Formatting-options overloads added where applicable — numeric types get
(string?, NumberStyles, IFormatProvider?), date/time types get(string?, IFormatProvider?, DateTimeStyles),TimeSpangets(string?, IFormatProvider?). The simple overloads delegate to these.boolandGuidhave no standard formatting parameters so they stay single-overload. - Tests only exercise the outermost
ParseOrNull(string?)overloads (unchanged).
Agent-Logs-Url: https://github.com/Tyrrrz/PowerKit/sessions/36a5377d-1796-4811-a4d0-0b4724b3541e Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
|
Instead of |
Agent-Logs-Url: https://github.com/Tyrrrz/PowerKit/sessions/3a5a128b-3368-43dd-90f7-237e6042cac7 Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
Done in 2d70ecf. All 9 host classes renamed ( |
There was a problem hiding this comment.
Pull request overview
Adds ParseOrNull static extension members for common value types in PowerKit.Extensions, providing a nullable-returning parse helper that defaults to InvariantCulture (where applicable) and returns null on unsuccessful parsing.
Changes:
- Introduces
ParseOrNull(string?)+ (where applicable) formatting-parameter overloads forint,long,double,decimal,DateTime,DateTimeOffset,TimeSpan, plus simpleboolandGuidvariants. - Adds per-type unit tests validating basic success/failure/null cases for the simple overloads.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| PowerKit/Extensions/BoolExtensions.cs | Adds bool.ParseOrNull(string?) wrapper over bool.TryParse. |
| PowerKit/Extensions/GuidExtensions.cs | Adds Guid.ParseOrNull(string?) wrapper over Guid.TryParse. |
| PowerKit/Extensions/IntExtensions.cs | Adds int.ParseOrNull (default + NumberStyles/IFormatProvider overload). |
| PowerKit/Extensions/LongExtensions.cs | Adds long.ParseOrNull (default + NumberStyles/IFormatProvider overload). |
| PowerKit/Extensions/DoubleExtensions.cs | Adds double.ParseOrNull (default + NumberStyles/IFormatProvider overload). |
| PowerKit/Extensions/DecimalExtensions.cs | Adds decimal.ParseOrNull (default + NumberStyles/IFormatProvider overload). |
| PowerKit/Extensions/DateTimeExtensions.cs | Adds DateTime.ParseOrNull (default + IFormatProvider/DateTimeStyles overload). |
| PowerKit/Extensions/DateTimeOffsetExtensions.cs | Adds DateTimeOffset.ParseOrNull (default + IFormatProvider/DateTimeStyles overload). |
| PowerKit/Extensions/TimeSpanExtensions.cs | Adds TimeSpan.ParseOrNull (default + IFormatProvider overload). |
| PowerKit.Tests/BoolExtensionsTests.cs | Adds basic tests for bool.ParseOrNull(string?). |
| PowerKit.Tests/GuidExtensionsTests.cs | Adds basic tests for Guid.ParseOrNull(string?). |
| PowerKit.Tests/IntExtensionsTests.cs | Adds basic tests for int.ParseOrNull(string?). |
| PowerKit.Tests/LongExtensionsTests.cs | Adds basic tests for long.ParseOrNull(string?). |
| PowerKit.Tests/DoubleExtensionsTests.cs | Adds basic tests for double.ParseOrNull(string?). |
| PowerKit.Tests/DecimalExtensionsTests.cs | Adds basic tests for decimal.ParseOrNull(string?). |
| PowerKit.Tests/DateTimeExtensionsTests.cs | Adds basic tests for DateTime.ParseOrNull(string?). |
| PowerKit.Tests/DateTimeOffsetExtensionsTests.cs | Adds basic tests for DateTimeOffset.ParseOrNull(string?). |
| PowerKit.Tests/TimeSpanExtensionsTests.cs | Adds basic tests for TimeSpan.ParseOrNull(string?). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [Fact] | ||
| public void ParseOrNull_Test() | ||
| { | ||
| // Act & assert | ||
| int.ParseOrNull("42").Should().Be(42); | ||
| int.ParseOrNull("-7").Should().Be(-7); | ||
| int.ParseOrNull("abc").Should().BeNull(); | ||
| int.ParseOrNull(null).Should().BeNull(); |
There was a problem hiding this comment.
This test only covers the simple ParseOrNull(string?) overload. Since IntExtensions also introduces the overload that accepts NumberStyles and IFormatProvider, please add at least one test that exercises those parameters (e.g., parsing a hex value with NumberStyles.HexNumber and InvariantCulture) to ensure the overload is wired correctly.
| [Fact] | ||
| public void ParseOrNull_Test() | ||
| { | ||
| // Act & assert | ||
| long.ParseOrNull("9876543210").Should().Be(9876543210L); | ||
| long.ParseOrNull("-1").Should().Be(-1L); | ||
| long.ParseOrNull("abc").Should().BeNull(); | ||
| long.ParseOrNull(null).Should().BeNull(); |
There was a problem hiding this comment.
This test only validates the simple ParseOrNull(string?) overload. LongExtensions also adds a NumberStyles/IFormatProvider overload, so it should have explicit test coverage (for example, a hex parse or a culture-specific thousands separator) to confirm the parameters are honored.
| [Fact] | ||
| public void ParseOrNull_Test() | ||
| { | ||
| // Act & assert | ||
| double.ParseOrNull("3.14").Should().Be(3.14); | ||
| double.ParseOrNull("-1.5").Should().Be(-1.5); | ||
| double.ParseOrNull("abc").Should().BeNull(); | ||
| double.ParseOrNull(null).Should().BeNull(); |
There was a problem hiding this comment.
This test suite covers only the default ParseOrNull(string?) behavior. DoubleExtensions also introduces the overload that accepts NumberStyles and IFormatProvider, so please add a test that depends on those parameters (e.g., a culture with ',' decimal separator or parsing a thousands-formatted number) to verify the overload works as intended.
| [Fact] | ||
| public void ParseOrNull_Test() | ||
| { | ||
| // Act & assert | ||
| decimal.ParseOrNull("3.14").Should().Be(3.14m); | ||
| decimal.ParseOrNull("-1.5").Should().Be(-1.5m); | ||
| decimal.ParseOrNull("abc").Should().BeNull(); | ||
| decimal.ParseOrNull(null).Should().BeNull(); |
There was a problem hiding this comment.
These assertions cover only the default ParseOrNull(string?) overload. Since DecimalExtensions adds a NumberStyles/IFormatProvider overload, please add a test that exercises that overload (for example, parsing a value with thousands separators or with a non-invariant culture) to ensure the formatting parameters are applied.
| [Fact] | ||
| public void ParseOrNull_Test() | ||
| { | ||
| // Act & assert | ||
| DateTime.ParseOrNull("2024-06-15").Should().Be(new DateTime(2024, 6, 15)); | ||
| DateTime.ParseOrNull("not a date").Should().BeNull(); | ||
| DateTime.ParseOrNull(null).Should().BeNull(); |
There was a problem hiding this comment.
This test covers only the simple ParseOrNull(string?) overload. DateTimeExtensions also adds an overload that accepts IFormatProvider and DateTimeStyles, so please add a test that requires non-default provider/styles (e.g., parsing a culture-specific date string) to validate that overload is correctly implemented.
| [Fact] | ||
| public void ParseOrNull_Test() | ||
| { | ||
| // Act & assert | ||
| DateTimeOffset.ParseOrNull("2024-06-15T00:00:00+00:00").Should().Be( | ||
| new DateTimeOffset(2024, 6, 15, 0, 0, 0, TimeSpan.Zero) | ||
| ); | ||
| DateTimeOffset.ParseOrNull("not a date").Should().BeNull(); | ||
| DateTimeOffset.ParseOrNull(null).Should().BeNull(); |
There was a problem hiding this comment.
This test only exercises the default ParseOrNull(string?) overload. DateTimeOffsetExtensions also adds an overload taking IFormatProvider and DateTimeStyles, so it needs explicit coverage (e.g., a culture-specific offset date/time format or a styles-dependent case) to ensure the overload parameters are actually used.
| [Fact] | ||
| public void ParseOrNull_Test() | ||
| { | ||
| // Act & assert | ||
| TimeSpan.ParseOrNull("1:30:00").Should().Be(new TimeSpan(1, 30, 0)); | ||
| TimeSpan.ParseOrNull("not a timespan").Should().BeNull(); | ||
| TimeSpan.ParseOrNull(null).Should().BeNull(); |
There was a problem hiding this comment.
This test validates only the invariant-culture default ParseOrNull(string?) overload. TimeSpanExtensions also introduces the overload that accepts an IFormatProvider, so please add at least one test that uses a non-invariant culture-dependent format to confirm the formatProvider parameter is respected.
|
@copilot the overloads without extra parameters should default to the current culture, to be consistent with the bae Parse method |
…e behavior Agent-Logs-Url: https://github.com/Tyrrrz/PowerKit/sessions/4a5577a4-6ae0-4ad3-9337-6bdf23cda70b Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
Done in 159aeaa. All 7 simple |
Adds
ParseOrNullas a static extension method on major value types, mirroring the pattern from GitHubActionsTestLogger. Each method takes astring?and returns a nullable of the target type, yieldingnullon failure.Types covered
int,longNumberStyles.Integer,CurrentCultureNumberStyles,IFormatProvider?doubleNumberStyles.Float | AllowThousands,CurrentCultureNumberStyles,IFormatProvider?decimalNumberStyles.Number,CurrentCultureNumberStyles,IFormatProvider?boolbool.TryParseDateTime,DateTimeOffsetCurrentCulture,DateTimeStyles.NoneIFormatProvider?,DateTimeStylesTimeSpanCurrentCultureIFormatProvider?GuidGuid.TryParseEach type (except
boolandGuid, which have no BCL formatting parameters) exposes a full overload accepting explicit formatting options. The simpleParseOrNull(string?)overload delegates to the full overload usingCultureInfo.CurrentCultureas the default, consistent with the behavior of the baseParsemethods.Usage
Structure
Each type has its own dedicated file and host class (e.g.
IntExtensions.cs/IntExtensions,LongExtensions.cs/LongExtensions, etc.), required because staticParseOrNulloverloads with the same signature would collide in a single host class under the current C# preview extension member semantics. Tests are organized into matching per-type files (e.g.IntExtensionsTests.cs,LongExtensionsTests.cs, etc.).