Clean up nested exceptions without value-add in ext/random#9211
Merged
TimWolla merged 7 commits intoAug 2, 2022
php:masterphp/php-src:masterfrom
TimWolla:random-exception-cleanup2TimWolla/php-src:random-exception-cleanup2Copy head branch name to clipboard
Merged
Clean up nested exceptions without value-add in ext/random#9211TimWolla merged 7 commits intophp:masterphp/php-src:masterfrom TimWolla:random-exception-cleanup2TimWolla/php-src:random-exception-cleanup2Copy head branch name to clipboard
TimWolla merged 7 commits into
php:masterphp/php-src:masterfrom
TimWolla:random-exception-cleanup2TimWolla/php-src:random-exception-cleanup2Copy head branch name to clipboard
Conversation
zeriyoshi
approved these changes
Aug 1, 2022
The only way that `php_binary_string_shuffle` fails is when the engine itself
fails. With the currently available list of engines we have:
- Mt19937 : Infallible.
- PcgOneseq128XslRr64: Infallible.
- Xoshiro256StarStar : Infallible.
- Secure : Practically infallible on modern systems.
Exception messages were cleaned up in phpGH-9169.
- User : Error when returning an empty string.
Error when seriously biased (range() fails).
And whatever Throwable the userland developer decides to use.
So the existing engines are either infallible or throw an Exception/Error with
a high quality message themselves, making this exception not a value-add and
possibly confusing.
Same reasoning as in the previous commit applies.
Same reasoning as in the previous commit applies.
Same reasoning as in the previous commit applies, except that it won't throw on a seriously biased user engine, as `range()` is not used.
Same reasoning as in the previous commit applies.
This implementation is shared across all native engines. Thus the same reasoning as the previous commits applies, except that the User engine does not use this method. Thus is only applicable to the Secure engine, which is the only fallible native engine.
be46f79 to
1b39d5e
Compare
Member
Author
|
Note: I'm currently working on a second PR that compliments this one and that adds a proper hierarchy for the remaining Exceptions (which are all useful on their own). |
Girgias
approved these changes
Aug 2, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.