Eliminates exceptions when characters are outside ISO-8859-1 range#672
Eliminates exceptions when characters are outside ISO-8859-1 range#672
Conversation
📝 WalkthroughWalkthroughReplaced exception-based ISO-8859-1 validation with a direct per-character range check in QRCoder/QRCodeGenerator.cs, removing a static Encoding fallback and associated GetByteCount call. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
301fa27 to
f50cc48
Compare
gfoidl
left a comment
There was a problem hiding this comment.
Avoiding the potential allocation is better than trading the few nano seconds here.
Speeding this up could be done by vectorized code, but it gets ugly and long -- especially it can't be implemented for all TFMs.
.NET (in recent versions) provides Ascii.IsValid (and Utf8.IsValid), which use a vectorized approach, so we could check for valid ascii (the majority of inputs will be ascii) first, and when not fallback to the manual loop. In the worst case that's
But for what? Just a few ns in trade for complicated code...
I'm fine with the solution you have here.
@gfoidl Although it only takes a few nanoseconds (billionths of a second) to run, it is 30% slower than the original code assuming all characters can be encoded. On the other hand, it is a thousand times faster with zero allocations for a string that requires UTF-8.
I could not find a more optimized approach.