Skip to content

#1593 fix Singapore ID number generation#1594

Merged
kingthorin merged 1 commit intomainfrom
fix/1593-singapore-id-number
Jul 22, 2025
Merged

#1593 fix Singapore ID number generation#1594
kingthorin merged 1 commit intomainfrom
fix/1593-singapore-id-number

Conversation

@asolntsev
Copy link
Copy Markdown
Collaborator

@asolntsev asolntsev commented Jul 22, 2025

It seems that faker.idNumber().singaporeanFin() generated an invalid NRIC (Singapore id number).

Implementation details:

SingaporeIdNumber.CODE array has length 8 but is used to compute pair-wise weighted sum with random digits of length 7. This commit removes the first element (0).

Examples:

I've tested results with online NRIC validator https://nric.biz/

Before the fix applied, we generated invalid NRICs:

G2315872X
G8075162M
G2597018R
G1313234T
G6608713U
G4203779P
G0702189P
G5571430U
G9666778Q
G9738269Q

After fix applied, we generate valid NRICs:

G4961953M
G3182038W
G3812929K
G4053384U
G7061899R
G3675833W
G2142968L
G5946754P
G5916294U
G8598112P

Fixes #1593

`SingaporeIdNumber.CODE` array has length 8 but is used to compute pair-wise weighted sum with random digits of length 7. This commit removes the first element (0).

I've tested results with online NRIC validator https://nric.biz/
@asolntsev asolntsev linked an issue Jul 22, 2025 that may be closed by this pull request
@asolntsev asolntsev added this to the 2.4.5 milestone Jul 22, 2025
@asolntsev asolntsev added the bug Something isn't working label Jul 22, 2025
@asolntsev asolntsev self-assigned this Jul 22, 2025
@what-the-diff
Copy link
Copy Markdown

what-the-diff bot commented Jul 22, 2025

PR Summary

  • Elimination of Unnecessary Import
    The "RandomService" was earlier incorporated in the file named "SingaporeIdNumber.java", but it wasn't in use. So, we've removed it to maintain the cleanliness of the code.

  • Modification in Code Logic
    In the code array within the "SingaporeIdNumber.java" file, we've replaced the first element from '0' to '2', thus enhancing the underpinning logic of the code.

  • Change in Random Digits Generation Method
    The approach to generating random numbers has been updated in the "generateValidIdNumber" function in the "SingaporeIdNumber.java" file. A new method from the Faker library, faker.number().randomDigits(7), replaces the previous custom randomDigits method, giving a more reliable and efficient way of generating random digits.

  • Removed Redundant Method
    We've made the code more compact by removing the randomDigits method from "SingaporeIdNumber.java" since it's no longer necessary after the recent change in the random digits generation method.

  • Addition of a New Method
    The "Number.java" file incorporates a fresh method randomDigits(int length). This can provide an array of random digits that match a pre-determined length, adding more flexibility to the numerical operations in the code.

  • New Test for Code Dependability
    A new parameterized test named "randomDigits" has been designed in "NumberTest.java". This test ensures the accurate and reliable operation of the new randomDigits(int length) method while sticking to the given specifications.

@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.43%. Comparing base (641ea02) to head (a6ec8df).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1594      +/-   ##
============================================
+ Coverage     92.32%   92.43%   +0.10%     
- Complexity     3354     3358       +4     
============================================
  Files           331      331              
  Lines          6620     6619       -1     
  Branches        655      655              
============================================
+ Hits           6112     6118       +6     
+ Misses          348      345       -3     
+ Partials        160      156       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@kingthorin kingthorin requested a review from Copilot July 22, 2025 08:43
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug in the Singapore ID number generation where the SingaporeIdNumber.CODE array had an incorrect length that caused invalid NRIC numbers to be generated. The fix ensures that generated Singapore ID numbers pass validation.

Key changes:

  • Corrected the CODE array by removing the first element to match the expected 7-digit format
  • Added a new randomDigits(int length) method to the Number class for generating arrays of random digits
  • Replaced the local randomDigits() method with the new generic implementation

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/main/java/net/datafaker/idnumbers/SingaporeIdNumber.java Fixed CODE array length and replaced local randomDigits method with generic implementation
src/main/java/net/datafaker/providers/base/Number.java Added new randomDigits method to generate arrays of random digits
src/test/java/net/datafaker/providers/base/NumberTest.java Added parameterized test for the new randomDigits method
Comments suppressed due to low confidence (1)

src/test/java/net/datafaker/providers/base/NumberTest.java:46

  • The test should include edge cases such as negative values to ensure the method handles invalid inputs appropriately.
    @ValueSource(ints = {0, 1, 2, 3, 4, 8, 16, 25, 36})

@kingthorin kingthorin merged commit d725193 into main Jul 22, 2025
13 checks passed
@kingthorin kingthorin deleted the fix/1593-singapore-id-number branch July 22, 2025 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Singapore NricNumber generation algorithm produces invalid value

4 participants