Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 4, 2025

The billion suffix generation used inconsistent code paths: position 3 fell through to a hardcoded return, while positions >= 6 used a formula-based approach. This made the logic harder to reason about and violated the principle of uniform handling.

Changes

  • Unified billion handling: Changed condition from positionFromRight >= 6 to positionFromRight >= 3, so all billion positions use Math.floor(positionFromRight / 3) to calculate repetitions
  • Constructor consistency: Replaced Array() with new Array() per project conventions
// Before: position 3 bypassed the formula
if (positionFromRight >= 6 && hasTrailingZeros) {
  const billionCount = Math.floor(positionFromRight / 3)
  return ' ' + Array(billionCount).fill('tỷ').join(' ')
}
return ' tỷ'  // position 3 hit this

// After: all positions use the same formula
if (positionFromRight >= 3 && hasTrailingZeros) {
  const billionCount = Math.floor(positionFromRight / 3)
  return ' ' + new Array(billionCount).fill('tỷ').join(' ')
}
return ' tỷ'  // only for non-trailing-zero cases

The behavior is unchanged (position 3 still produces ' tỷ'), but the implementation is now consistent across all billion positions.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 4, 2025

Copilot AI changed the title [WIP] Update Vietnamese numeral reading and configuration based on feedback Fix billion suffix logic to handle all positions >= 3 consistently Nov 4, 2025
Copilot AI requested a review from hckhanh November 4, 2025 07:20
Copilot finished work on behalf of hckhanh November 4, 2025 07:20
@hckhanh hckhanh marked this pull request as ready for review November 4, 2025 07:21
Copilot AI review requested due to automatic review settings November 4, 2025 07:21
@hckhanh hckhanh merged commit 21d3311 into fix-billion Nov 4, 2025
7 checks passed
@hckhanh hckhanh deleted the copilot/sub-pr-51 branch November 4, 2025 07:21
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 fixes a logic issue in the billion suffix generation for Vietnamese number reading and improves code consistency. The change corrects the condition for when to apply the repeated "tỷ" suffix pattern.

  • Fixed the condition threshold from >= 6 to >= 3 for generating repeated billion suffixes
  • Updated array instantiation to use new Array() for consistency

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

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