Skip to content

Conversation

@hckhanh
Copy link
Owner

@hckhanh hckhanh commented Nov 3, 2025

This pull request introduces a major refactor of the Vietnamese number reading module, shifting from an object-oriented approach to a functional, modular design for improved performance, maintainability, and code quality. The public API remains unchanged and fully backward compatible, but the internal structure has been completely rewritten, resulting in significant performance gains, reduced complexity, and better test coverage. Additionally, there is a minor optimization to the formatNumber function.

Major Refactor: Functional Rewrite and Modularization

  • Entirely replaced the class-based architecture (NumberReader, Numbers, Thousand, Million, Billion, Zerofill and related files) with a pure functional approach, removing inheritance and dynamic dispatch for better performance and maintainability. The internal logic is now split into five focused modules: digits.ts, utils.ts, three-digits.ts, groups.ts, and index.ts. Public API (readVnNumber) remains unchanged. (.changeset/late-wings-add.md, [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]
  • Removed 12 obsolete files (old class implementations and their tests) and added 4 new modular files with clear separation of concerns. (.changeset/late-wings-add.md)

Performance and Code Quality Improvements

  • Eliminated class instantiation, property access overhead, and inheritance chain lookups, resulting in faster function calls and more efficient string operations. Benchmarks show maintained or improved performance (2.3-6.8M ops/sec). (.changeset/late-wings-add.md)
  • Refactored complex functions (readThreeDigits, readVnNumber) to reduce cognitive complexity below SonarCloud thresholds, improving readability and maintainability. (.changeset/late-wings-add.md)

Testing Enhancements

  • Expanded and improved test coverage: increased from 42 to 68 tests (+62%), with comprehensive edge case coverage (leading zeros, tens, hundreds, mixed magnitudes, boundary values, and all Vietnamese reading rules). All 91 tests now pass. (.changeset/late-wings-add.md)

Minor Function Optimization

  • Optimized the logic of the formatNumber function in src/format/number.ts for early returns and clearer handling of null, bigint, and number types. (.changeset/wide-things-dig.md, src/format/number.tsL13-R19)

Copilot AI review requested due to automatic review settings November 3, 2025 08:22
@codecov-commenter
Copy link

codecov-commenter commented Nov 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (239c79a) to head (c603fc2).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##             main       #43      +/-   ##
===========================================
+ Coverage   93.02%   100.00%   +6.97%     
===========================================
  Files          12         6       -6     
  Lines         258       245      -13     
  Branches       68        82      +14     
===========================================
+ Hits          240       245       +5     
+ Misses         15         0      -15     
+ Partials        3         0       -3     

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
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 refactors the formatNumber function to use Number.isNaN() instead of the global isNaN() function for NaN checking. This is a best practice improvement that avoids the type coercion issues associated with the global isNaN() function.

Key Changes:

  • Replaced isNaN() with Number.isNaN() for checking NaN values in the number validation logic

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 3, 2025

CodSpeed Performance Report

Merging #43 will degrade performances by 85.15%

Comparing optimized (c603fc2) with main (239c79a)

Summary

⚡ 20 improvements
❌ 1 regression
✅ 43 untouched

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
format order count (1,234) 31.7 µs 28.6 µs +10.65%
formatVnNumber with different input types 429.2 µs 45.4 µs ×9.5
bigint input (large transaction) 174 µs 43.3 µs ×4
mixed input types in data processing (30 values) 659.2 µs 272.4 µs ×2.4
number input (typical price) 169.5 µs 39.5 µs ×4.3
string input (from API/form) 172 µs 49.3 µs ×3.5
display financial dashboard (20 metrics) 468 µs 204 µs ×2.3
display invoice line items (10 items) 519.1 µs 183.7 µs ×2.8
display payment receipt (typical transaction) 193 µs 50.9 µs ×3.8
display product catalog (20 products) 739.1 µs 298.6 µs ×2.5
display shopping cart (5 items with quantities) 299.9 µs 120 µs ×2.5
read 50 e-commerce prices (10k-50M VND) 1,409.9 µs 369 µs ×3.8
read 50 financial amounts (1M-10B VND) 1,182 µs 465.4 µs ×2.5
read 50 invoice totals (100k-100M VND) 903.6 µs 362.6 µs ×2.5
read 50 product quantities (1-100) 566.3 µs 3,814.3 µs -85.15%
annual revenue (2,500,000,000 VND) 183 µs 53.8 µs ×3.4
invoice total (5,450,000 VND) 171.5 µs 57.8 µs ×3
large contract value (15,000,000,000 VND) 167.3 µs 49.4 µs ×3.4
product quantity (5 items) 86 µs 66.7 µs +28.89%
transaction amount (19,990,000 VND) 177.9 µs 53 µs ×3.4
... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

@hckhanh hckhanh force-pushed the optimized branch 2 times, most recently from 0a30fb3 to 962e996 Compare November 3, 2025 09:29
hckhanh and others added 7 commits November 3, 2025 16:42
Replaced `isNaN` with `Number.isNaN` to improve type safety and clarity. Ensures more predictable behavior when handling `NaN` values across different types. Updated fallback logic to conform to the new check.
Removed redundant classes and consolidated the Vietnamese number reading process into a single function. Streamlined implementation improves maintainability, reduces code complexity, and eliminates duplication across modules. Updated tests and adjusted configurations accordingly.
Added exclusions for `tsdown.config.*` in test and coverage settings within the Vitest configuration. This ensures that unnecessary files are not included in tests or coverage reports, improving accuracy and performance.
Added extensive test cases for various number patterns, including leading zeros, tens, hundreds, and boundary values. These improvements enhance the coverage and reliability of the Vietnamese number reading functionality. Updated logic to handle corner cases more accurately.
Refactored `formatNumber` to reduce redundancy and improve readability. Combined conditions to streamline the implementation and adjusted handling of null and string inputs. Updated fallback behavior to maintain correctness and consistency.

test(config): refine Vitest exclusions

Added specific exclusions for `src/index.ts` and `src/format/index.ts` in both test and coverage settings. Improves test accuracy by omitting irrelevant files from coverage metrics.
@hckhanh hckhanh changed the title Optimize performance Modularize readVnNumber function instead of using classes Nov 3, 2025
hckhanh and others added 5 commits November 3, 2025 16:57
Added badges for publish workflow, quality gate status, and CodSpeed performance. Enhances README visualization and provides quick access to project metrics.
Refactored number reading logic by introducing separate functions for first and subsequent groups, improving modularity and readability. Removed redundant logic and reorganized code for better maintainability.
Added support for BigInt values in number formatting tests, ensuring compatibility across formats. Introduced comprehensive test cases for Vietnamese number reading, covering edge cases, groups, and suffix handling for enhanced accuracy and reliability.
@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 3, 2025

@hckhanh hckhanh merged commit 4b7c059 into main Nov 3, 2025
6 of 7 checks passed
@hckhanh hckhanh deleted the optimized branch November 3, 2025 10:07
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.

3 participants