Skip to content

[en_GB] VAT number faker#255

Merged
pimjansen merged 7 commits intoFakerPHP:mainfrom
dmlogic:main
Jan 8, 2021
Merged

[en_GB] VAT number faker#255
pimjansen merged 7 commits intoFakerPHP:mainfrom
dmlogic:main

Conversation

@dmlogic
Copy link
Copy Markdown

@dmlogic dmlogic commented Jan 7, 2021

What is the reason for this PR?

  • A new feature

Author's checklist

Summary of changes

  • Added a en_GB\Company provider to fake VAT numbers
  • Documentation PR is here

Review checklist

  • All checks have passed
  • Changes are approved by maintainer

Comment thread src/Faker/Provider/en_GB/Company.php Outdated
Comment thread src/Faker/Provider/en_GB/Company.php Outdated
Comment thread test/Faker/Provider/en_GB/CompanyTest.php Outdated
Comment thread src/Faker/Provider/en_GB/Company.php Outdated
return sprintf(
'%s%d %d %d %d',
static::VAT_PREFIX,
static::randomNumber(3, true),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only produces numbers bigger than 100. Is that on purpose? If not you could use the str_pad solution from below here too. The other cases that use randomNumber have the same issue.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I'd rather stick with the simpler randomNumber() generator where possible. If I was wanting to test the behaviour of numbers with leading zeros I would use a defined value rather than faking it.

Copy link
Copy Markdown

@krsriq krsriq Jan 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also use static::numerify('### #### ## ###').

Comment thread src/Faker/Provider/en_GB/Company.php Outdated
@dmlogic
Copy link
Copy Markdown
Author

dmlogic commented Jan 8, 2021

I re-read the Wikipedia notes about this and have subsequently re-worked things a bit. Numbers are now much more likely to pass validation checks and hopefully some of the other issues raised have gone away.

Comment thread test/Faker/Provider/en_GB/CompanyTest.php Outdated
Comment thread src/Faker/Provider/en_GB/Company.php
@bram-pkg bram-pkg changed the title UK VAT number faker [en_GB] VAT number faker Jan 8, 2021
@pimjansen pimjansen added the enhancement New feature or request label Jan 8, 2021
@bram-pkg bram-pkg requested a review from pimjansen January 8, 2021 15:45
@pimjansen pimjansen merged commit 7e6c6b0 into FakerPHP:main Jan 8, 2021
@dmlogic
Copy link
Copy Markdown
Author

dmlogic commented Jan 8, 2021

Thank you

@lukasjuhas lukasjuhas mentioned this pull request Mar 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants