Bogus icon indicating copy to clipboard operation
Bogus copied to clipboard

Added support for Vietnamese Citizen Identity Card Number

Open git03-Nguyen opened this issue 1 year ago • 4 comments

Hello, this PR added extension method to generate Vietnamese 12-digit Citizen Identity Card Number ("Căn cước công dân - CCCD" in local language):

  • Add extension method Cccd() on Person object in Bogus.Extensions.Vietnam namespace.
  • Add unit tests.
  • Modify README.md.

git03-Nguyen avatar Nov 02 '24 10:11 git03-Nguyen

Great work. Thank you for this pull request. I'll try to get to this soon, but might be a little difficult with the holidays coming up, I'll be at the airport in a few days and hopefully I will try to review this while I wait at the terminal

bchavez avatar Nov 05 '24 03:11 bchavez

Great work. Thank you for this pull request. I'll try to get to this soon, but might be a little difficult with the holidays coming up, I'll be at the airport in a few days and hopefully I will try to review this while I wait at the terminal

Happy to hear that ^^. It's just some simple work done in my spare time cuz I really love Bogus. Anw, tell me if the code needs any changes.

git03-Nguyen avatar Nov 05 '24 04:11 git03-Nguyen

Hi @git03-Nguyen; thanks again for the PR.

After reviewing this PR at the airport, it appears we are slightly misaligned with some of city names which could potentially throw some exceptions when this code executes.

I diffed the following vi locale city names from our JSON:

  • https://github.com/bchavez/Bogus/blob/905a7e753ed2d7bfd42fc5099a50278adf45b467/Source/Bogus/data/vi.locale.json#L36

then compared it to the CityCodes in this PR, and we have a few city names that look like they might not match;

image

image

We should probably get these two lists corrected and synchronized so we don't stumble on an exception with throw new ArgumentException($"City {p.Address.City} is not supported." when Person.Cccd() executes.

Let me know what you think.

bchavez avatar Nov 08 '24 11:11 bchavez

Hi @bchavez , thank you so much for your review, and I apologize for my negligence. Actually, I did compare the lists initially and fixed some differences, but I only gave them a quick glance, it was my mistake not using proper diff tools.

I’ve now pushed two commits:

  • The first fixes the city names "Khánh Hoà", "Thừa Thiên-Huế", and "Thanh Hoá", and includes related unit test cases.
  • The second generates the city names using the Bogus data "vi.locale.json" and adds exhaustive tests for all 63 cities/provinces in Vietnam. If you feel it’s unnecessary, we can revert that commit.

Please let me know if there’s anything else you'd like me to adjust. I really appreciate your feedback! <3

git03-Nguyen avatar Nov 09 '24 03:11 git03-Nguyen