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

$code should have (string) type. (int) will produce always false. #95

Closed
wants to merge 1 commit into from
Closed

Conversation

utilmind
Copy link

@utilmind utilmind commented Nov 7, 2022

Hello! I just spent some amount of time trying to figure out why it doesn't accepts entered 6-digits integer then realized that verifyCode() only accepts (string)'s.

I suggest to make strict conversion $code to (string), to correctly process possible integers.

Also I suggest to replace $var++ to ++$var for better performance, but it's less important.

Thanks!

@RobThree
Copy link
Owner

RobThree commented Nov 7, 2022

The code should never be an int, it's always a string in the first place. An integer can't hold zeroes: 008164 in an int becomes 8614 which 1) doesn't match the length requirement and 2) is ambigious; did the user forget to input the missing digits or did the user, somehow, get 4 digits from somewhere?

When we drop support for old(er) PHP versions all methods (including verifyCode() will be type hinted (i.e. verifyCode(string $secret, string $code, int $discrepancy = 1, ...).

The $var++ v.s. ++$var is such a micro-optimisation (if at all) on such little iterations that it will make no noticeable difference in whatever real world setting.

Thanks for your input and PR anyway! It may not sound like it, but it's appreciated nonetheless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants