Skip to content

Simplify NumberUtils.isParsable(String)#1570

Merged
garydgregory merged 1 commit intoapache:masterfrom
yuanchaodu:fix-NumberUtils-isParsable
Jan 18, 2026
Merged

Simplify NumberUtils.isParsable(String)#1570
garydgregory merged 1 commit intoapache:masterfrom
yuanchaodu:fix-NumberUtils-isParsable

Conversation

@yuanchaodu
Copy link
Copy Markdown
Contributor

@yuanchaodu yuanchaodu commented Jan 16, 2026

No description provided.

garydgregory
garydgregory reviewed Jan 16, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@garydgregory garydgregory left a comment •
edited
Loading

Choose a reason for hiding this comment

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

How does this not match the Javadoc? The order in the code is intentional as Double will parse almost everything.

@yuanchaodu
Copy link
Copy Markdown
Contributor Author

yuanchaodu commented Jan 18, 2026

How does this not match the Javadoc? The order in the code is intentional as Double will parse almost everything.

duplicate Long::parseLong

@garydgregory
Copy link
Copy Markdown
Member

garydgregory commented Jan 18, 2026

@yuanchaodu
Ah, ok, I see it now, thank you. As I wrote, don't change the order.

@yuanchaodu yuanchaodu force-pushed the fix-NumberUtils-isParsable branch 2 times, most recently from b1342ff to ea44bdb Compare January 18, 2026 11:43
@yuanchaodu
Copy link
Copy Markdown
Contributor Author

yuanchaodu commented Jan 18, 2026

@yuanchaodu Ah, ok, I see it now, thank you. As I wrote, don't change the order.

Please review again. @garydgregory

@garydgregory
Copy link
Copy Markdown
Member

garydgregory commented Jan 18, 2026

@yuanchaodu
Hm, if parseLong succeeds, then do you really need parseInt?

@yuanchaodu yuanchaodu force-pushed the fix-NumberUtils-isParsable branch from ea44bdb to 13833eb Compare January 18, 2026 12:25
@yuanchaodu
Copy link
Copy Markdown
Contributor Author

yuanchaodu commented Jan 18, 2026

@yuanchaodu Hm, if parseLong succeeds, then do you really need parseInt?

Done. @garydgregory

@yuanchaodu yuanchaodu changed the title Fix NumberUtils.isParsable implementation to match its Javadoc @yuanchaodu Fix NumberUtils.isParsable duplicate Long::parseLong Jan 18, 2026
@yuanchaodu yuanchaodu changed the title @yuanchaodu Fix NumberUtils.isParsable duplicate Long::parseLong Fix NumberUtils.isParsable duplicate Long::parseLong Jan 18, 2026
@garydgregory
Copy link
Copy Markdown
Member

garydgregory commented Jan 18, 2026

@yuanchaodu
And obviously, my next question: If parseDouble succeeds, then do you really need parseFloat?

@yuanchaodu yuanchaodu force-pushed the fix-NumberUtils-isParsable branch from 13833eb to a372bd2 Compare January 18, 2026 12:36
@yuanchaodu
Copy link
Copy Markdown
Contributor Author

yuanchaodu commented Jan 18, 2026

@yuanchaodu And obviously, my next question: If parseDouble succeeds, then do you really need parseFloat?

Done.

@garydgregory garydgregory changed the title Fix NumberUtils.isParsable duplicate Long::parseLong Simplify NumberUtils.isParsable(String) Jan 18, 2026
@garydgregory garydgregory merged commit 386af29 into apache:master Jan 18, 2026
65 of 84 checks passed
garydgregory added a commit that referenced this pull request Jan 18, 2026
@garydgregory
Simplify NumberUtils.isParsable(String) #1570
865a029
@yuanchaodu yuanchaodu deleted the fix-NumberUtils-isParsable branch January 22, 2026 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@garydgregory garydgregory Awaiting requested review from garydgregory

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@yuanchaodu @garydgregory