Skip to content
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

Refactor compatibility extension methods #534

Merged
merged 3 commits into from
May 24, 2024

Conversation

Shane32
Copy link
Contributor

@Shane32 Shane32 commented May 22, 2024

Summary

Refactors the Stream.CopyTo method into a polyfill for .NET 3.5 only, and has IsNullOrWhiteSpace inline to the native implementation for available platforms. These methods are internal and the changes will not affect operation.

In addition, ReverseString and IsAllDigit methods were removed as they were not referenced within the codebase.

Test Plan

Existing tests cover both .NET 3.5 and other platforms, ensuring compatibility. The changed and removed methods are not publicly accessible.

Copy link
Owner

@codebude codebude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we rewrite/use the IsNullOrWhiteSpace function as an extension method in the same manner as CopyTo? Currently at least in the PayloadGenerator.cs the calls aren't looking nice...

if (String40Methods.IsNullOrWhiteSpace(Secret))

if (Secret.IsNullOrWhiteSpace()) would look/feel smoother.

I'm sorry to mention such small things. But if we're going to tackle the points, I prefer to do it 100%. If I get too picky, let me know. Then I slow myself down a little. ;-)

@Shane32
Copy link
Contributor Author

Shane32 commented May 24, 2024

Certainly. I’ll double check and see. I know the native method is not an extension method. So we can’t just polyfill it like CopyTo. But we could alter the name slightly if nothing else. I’ll check.

@Shane32
Copy link
Contributor Author

Shane32 commented May 24, 2024

@codebude Looks like it works as anticipated !

@codebude codebude merged commit 6b7311b into codebude:master May 24, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants