Skip to content

Refactor small maintainability issues in transformations, fake-value resolution and Irish PPSN generation#1871

Open
tawratnibir wants to merge 5 commits into
datafaker-net:mainfrom
tawratnibir:refactor/phase3-int05
Open

Refactor small maintainability issues in transformations, fake-value resolution and Irish PPSN generation#1871
tawratnibir wants to merge 5 commits into
datafaker-net:mainfrom
tawratnibir:refactor/phase3-int05

Conversation

@tawratnibir

Copy link
Copy Markdown

Summary

This pull request makes a small set of maintainability-focused refactorings:

  • Replaces raw Class usage in JavaObjectTransformer.apply() with Class<?> and typed instanceof handling.
  • Removes an unreachable dotIndex == -1 branch from the private FakeValuesService.resolveFakerObjectAndMethod() helper.
  • Removes a misleading expression-resolution comment that referred to indexOf() while the code uses contains("}").
  • Extracts the Irish PPSN weighted checksum calculation into a private helper method.

Why

These changes address small maintainability issues found during source inspection. They improve type clarity, remove unreachable control-flow noise, align comments with the current implementation, and separate checksum calculation from the high-level PPSN generation workflow.

The changes are intentionally small and avoid public API or generic-contract redesign.

Refactoring approach

The refactoring was kept local to the affected implementation details:

  • JavaObjectTransformer.apply() now uses Class<?> to represent an unknown class type more safely.
  • FakeValuesService.resolveFakerObjectAndMethod() now reflects the verified private helper call contract more directly.
  • FakeValuesService.resolveExpression() no longer contains a stale implementation-specific comment.
  • IrishIdNumber.generateValid() now delegates weighted checksum accumulation to calculateWeightedSum(...), while preserving the existing weights, suffix handling, multiplier, checksum character calculation and output format.

Validation

The following validation was performed:

  • JavaObjectTransformerTest and JavaObjectTransformerConstructorTest passed.
  • FakeValuesServiceTest passed.
  • IrishIdNumberTest passed.
  • Full Maven verification passed with:
./mvnw -Dgpg.skip=true clean verify

@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

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.52%. Comparing base (2f057eb) to head (7383fed).
⚠️ Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
...tafaker/transformations/JavaObjectTransformer.java 50.00% 0 Missing and 1 partial ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1871      +/-   ##
============================================
+ Coverage     92.45%   92.52%   +0.06%     
- Complexity     3557     3563       +6     
============================================
  Files           345      346       +1     
  Lines          7025     7050      +25     
  Branches        685      685              
============================================
+ Hits           6495     6523      +28     
+ Misses          366      364       -2     
+ Partials        164      163       -1     

☔ View full report in Codecov by Harness.
📢 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants