Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

Fix bug in numberOfIntegerDigits calculation, simplifying the algorithm #535

Merged
merged 3 commits into from
Jan 23, 2023

Conversation

mosuem
Copy link
Contributor

@mosuem mosuem commented Jan 20, 2023

This fixes a bug where the number of digits may be wrong due to rounding errors.

@mosuem mosuem requested a review from devoncarew January 20, 2023 15:03
@mosuem mosuem self-assigned this Jan 20, 2023
Copy link
Contributor

@devoncarew devoncarew left a comment

Choose a reason for hiding this comment

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

lgtm w/ the indicated change related to the file header

return max(1, (log(simpleNumber) / _ln10).ceil());
if (simpleNumber < 100000000000000000) return 17;
if (simpleNumber < 1000000000000000000) return 18;
return 19;
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume we're not expecting anything past 19?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Anything larger cannot be represented in 64 bits, so we are safe!

@@ -0,0 +1,12 @@
import 'package:intl/intl.dart';
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add a copyright header for this file

@copybara-service copybara-service bot merged commit 3fcc810 into master Jan 23, 2023
@copybara-service copybara-service bot deleted the changeNumberOfDigitsCalculation branch January 23, 2023 15:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants