Skip to content

add CountryUtils#1459

Closed
jack5505 wants to merge 5 commits intoapache:masterfrom
jack5505:countryUtils
Closed

add CountryUtils#1459
jack5505 wants to merge 5 commits intoapache:masterfrom
jack5505:countryUtils

Conversation

@jack5505
Copy link
Copy Markdown

@jack5505 jack5505 commented Oct 5, 2025

Thanks for your contribution to Apache Commons! Your help is appreciated!

Before you push a pull request, review this list:

  • Read the contribution guidelines for this project.
  • Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
  • I used AI to create any part of, or all of, this pull request.
  • Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best-practice.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.

Copy link
Copy Markdown
Member

@garydgregory garydgregory left a comment

Choose a reason for hiding this comment

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

Hello @jack5505

Thank you for your PR.

-1 As it the PR stands now. There is duplication with what Java provides as well as Lang's own LocaleUtils. For example, using plain Java:

assertEquals("USA", Locale.US.getISO3Country());

Using LocaleUtils:

assertEquals("USA", LocaleUtils.languagesByCountry("US").get(0).getISO3Country());

As far as phone number data, this likely doesn't belong in a lower-level library like Commons Lang any more than postal code data, but it might belong in Commons Validator. It might be worth discussing finding a home for phone data on the mailing list.

That said, it is likely that we can improve LocaleUtils to achieve some of your use cases.

The changes to FieldUtilsTest.java and StringUtilsTest.java should be reverted as they have nothing to do with the subject of the PR.

Always run mvn by itself and fix all issues before pushing. Right now, all builds fail.

Thank you.

@jack5505
Copy link
Copy Markdown
Author

jack5505 commented Oct 5, 2025

Hi thank you for feedback. I will revert not relating part FieldUtilsTest.java and StringUtilsTest.java. But in your LocaleUtils there is no numeric code of country which is give a little bit convenient.

@garydgregory
Copy link
Copy Markdown
Member

Hello @jack5505

Here are a couple of comments and updates:

  • Please rebase on git master.
  • The getIso3(String) API in this PR is now redundant, you can now call:
    • LocaleUtils.toLocale("FR").getISO3Country(), and
    • LocaleUtils.toLocale("FR").getDisplayCountry()
  • The existing API LocaleUtils.toLocal(String) has been updated to work with 2-letter country codes without using any hard-coded tables.
  • This test in this PR is wrong:
    assertEquals("United States of America", CountryUtils.getCountryName("US")); because Locale.getDisplayCountry() for the US Locale returns "United States", not "United States of America".

@jack5505 jack5505 closed this Oct 6, 2025
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