Skip to content

Conversation

@stevenaw
Copy link
Member

@stevenaw stevenaw commented Feb 15, 2021

Contributes to #3588

This PR tackles the low-hanging fruit of StartsWith/EndsWith/Contains of string comparisons within the framework. It focuses on cases where the argument is a string literal with all characters lying within the standard ASCII range.

The majority of the cases deal with either:

  • Inspection of type/member names
  • Detection of line breaks

@stevenaw stevenaw changed the title Update internal string comparisons to use Ordinal when safe WIP: Update internal string comparisons to use Ordinal when safe Feb 15, 2021
@stevenaw stevenaw force-pushed the 3588/ordinal-string-comparisons branch from ddaee9f to aa65adf Compare February 16, 2021 00:43
@stevenaw stevenaw changed the title WIP: Update internal string comparisons to use Ordinal when safe Update internal string comparisons to use Ordinal when safe Feb 16, 2021
Copy link
Member

@mikkelbu mikkelbu left a comment

Choose a reason for hiding this comment

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

Looks good @stevenaw. I've added some suggestions, but overall it looks sound.

Co-authored-by: Mikkel Nylander Bundgaard <mikkelbu@users.noreply.github.com>
@stevenaw
Copy link
Member Author

Oh, thanks @mikkelbu ! Using the char overload makes sense, I'm not sure how I missed that the first time

@stevenaw
Copy link
Member Author

@mikkelbu Thanks again for your review. I've tried updating the single-length string StartsWith/EndsWith to use the char overload, but I think that's only a netstandard 2.1+ API. I receive a compiler error on the current build targets of net45 + netstandard2.

@mikkelbu
Copy link
Member

@stevenaw You are right (just took at look at the documentation). I'll merge this now. Thanks for the contribution 👍

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