Modernize code #9
Conversation
|
I have a follow-up PR ready to improve the library even further, but it's a "very" breaking change, bumping minimum PHP requirements to 8.1 (7.4 is EOL after all) and using more modern PHP features. I'll opened it as a draft for discussions once (if) this PR is merged. Alternatively I could incorporate the changes into this PR, if you decide to not support EOL PHP any more. I'm curious about your feedback and especially how you'd handle backwards incompatible changes. |
Uses types where possible and improves array handling. The default challenge options now use sane defaults and optional parameters. Improves validation of user input.
|
This PR is amazing |
| * | ||
| * @return Challenge The challenge data to be passed to ALTCHA. | ||
| */ | ||
| public static function createChallenge(BaseChallengeOptions $options): Challenge |
There was a problem hiding this comment.
Intuitively, using a base class, BaseChallengeOptions, instead of an interface, e.g., ChallengeOptionsInterface doesn't make the code easier to read and still doesn't tell the user which class to instantiate. So I opted for a base class here.
| /** | ||
| * @phpstan-type ChallengeParams array<string, null|scalar> | ||
| */ | ||
| class BaseChallengeOptions |
There was a problem hiding this comment.
I'm using a base class here because in PHP < 8 it's impossible to skip default parameters if you want specify another one.
That's the reason I implemented two child classes with different parameters (ordering) to allow a set of defined values, omitting those that should never be specified by users anyway.
In PHP > 8 I could have used a union type instead (no common base class for ChallengeOptions and CheckChallengeOptions) but the parameters are basically the same anyway and I didn't want to completely rewrite how challenge validation works.
|
Thanks for the PR, great work and very appreciated! I think it's okay to switch to php 8, if anybody need php 7, they can use the old version. I thinks it's better to release the new version after the other PR #10 is complete since you already started on that. |
This PR refs #8 to improve validation and use types (where possible).
The most important breaking change is the replacement of creating a challenge using arrays with typed object properties.
This type safety instantly makes the code easier to use and read (it's clear which type the parameters should have and how they should be used).
All public methods are now documented with PHPDoc to improve dev experience in modern IDEs.