use Luhn to calculate ar_SA id numbers. #875
Conversation
| public static function companyIdNumber() | ||
| { | ||
| do { | ||
| $number = static::numerify('700#######'); |
There was a problem hiding this comment.
nah, you can do better than try until it's valid!
There was a problem hiding this comment.
Actually this is exactly what is done on the Person provider for the same locale, but I will look for something.
There was a problem hiding this comment.
Luhn::computeCheckDigit() will give you the correct last digit.
There was a problem hiding this comment.
Then the Person provider is wrong, too, and you can update it while you're touching Luhns.
There was a problem hiding this comment.
I think I did on commit 757750f.
There was a problem hiding this comment.
I don't want to sound stupid, but I see the changes on the last commit on this PR.
| class Utils | ||
| { | ||
| // ripped unashamedly from https://github.com/grahamking/darkcoding-credit-card/blob/master/gencc.php | ||
| public static function generateLuhnNumber($prefix, $length) |
There was a problem hiding this comment.
You already have everything in Faker\Calculator\Luhn. No need to reimplement it here.
There was a problem hiding this comment.
Is it good enough now?
|
|
||
| class Utils | ||
| { | ||
| public static function generateLuhnNumber($prefix, $length) |
There was a problem hiding this comment.
this would be better placed in Faker\Calculator\Luhn
|
Anything new about this pull request? |
| } | ||
|
|
||
| $pattern = $prefix . str_repeat('#', $length - strlen($prefix) - 1); | ||
| $partialValue = BaseProvider::numerify($pattern); |
There was a problem hiding this comment.
you should pass the partialValue as parameter to this method, to separate the responsibilities: Luhn calculation (this class), random generation (Company.php)
|
I have made the changes as I understand how they should be done. Please check. |
|
Excellent, thanks for your contribution! |
|
Thank you too for help me through it. |
No description provided.