Refactor enum handling for improved performance in targets that do not have ReadOnlySpan<char> support. #153
Conversation
…t have ReadOnlySpan<char> support. Updated `SourceGenerationHelper` to use `global::System.MemoryExtensions.AsSpan` for string comparisons, enhancing performance and consistency. Removed conditional compilation directives for .NET Core versions, streamlining the codebase. Added a package reference to `System.Memory` version `4.6.3` in the project file. Updated multiple test files in the `EnumGeneratorTests` namespace to align with the new implementation, ensuring valid and efficient tests. Overall, these changes simplify string handling in enum generation and validation.
| @@ -18,5 +18,6 @@ | |||
|
|
|||
| <ItemGroup> | |||
| <PackageReference Include="System.ComponentModel.Annotations" Version="5.0.0" /> | |||
| <PackageReference Include="System.Memory" Version="4.6.3" /> | |||
There was a problem hiding this comment.
tl;dr; this is the breaking change that I don't want to introduce 😄
There was a problem hiding this comment.
What if it's only for the targets that benefit from that? Chances are users are already using that.
There was a problem hiding this comment.
I just personally don't really want the complexity🤷♂️ You don't get ReadOnlySpan<T> support in the built-in Enum.ToString() etc for NetStandard, and it breaks for people who are currently using the library in .NET Standard and aren't using NetStandard. Also means you can no longer use it in SourceGenerators for example (because adding those packages is a world of pain)
There was a problem hiding this comment.
I get in all the good places.
Right now, I'm copying the generated code when I need .NET Standard 2.0 support.
There was a problem hiding this comment.
You can keep it as is for now and later introduce static extension methods to support implicit conversions to and from string and ReadOnlySpan<char> when targeting the .NET 10 SDK. That said, we should bear in mind that .NET 8 will remain supported for nearly 18 more months.
There was a problem hiding this comment.
You can keep it as is for now and later introduce static extension methods to support implicit conversions to and from
stringandReadOnlySpan<char>when targeting the .NET 10 SDK. That said, we should bear in mind that .NET 8 will remain supported for nearly 18 more months.
I really don't follow? 😅 Definitely don't want implicit conversions from enums to string, that's not what you mean, right? And yeah, if you don't have the .NET 10 SDK then you just don't get the extension member versions. Not sure what that has to do with this PR though? 😅
There was a problem hiding this comment.
Not from and to an enum to string. From and to string and ReadOnlySpan<char> for .NET Standard 2.0. And that would be a file scoped extension.
Updated
SourceGenerationHelperto useglobal::System.MemoryExtensions.AsSpanfor string comparisons, enhancing performance and consistency. Removed conditional compilation directives for .NET Core versions, streamlining the codebase. Added a package reference toSystem.Memoryversion4.6.3in the project file. Updated multiple test files in theEnumGeneratorTestsnamespace to align with the new implementation, ensuring valid and efficient tests. Overall, these changes simplify string handling in enum generation and validation.Closes #152