-
-
Notifications
You must be signed in to change notification settings - Fork 184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create to fixed function #551
Create to fixed function #551
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Though it would be good to reduce the amount of code here. I'd like to always return a string too rather than a result: we can treat precision less than 0 as 0.
I think we could simplify by using one of these approaches:
- float-to-string and then recursively iterate on the string, pattern matching until the
.
is found and then pattern matching the number of desired characters. - float-to-string and then split on
.
and take the number of desired characters - use division and modulo to get the number left and right of the
.
and then convert those ints to strings and join with a.
.
No new externals should be needed.
Could you add a test for a precision of 0
and -1
too please?
Thank you.
/// to_string_fixed(0.3, 3) | ||
/// // -> Ok("0.300") | ||
/// | ||
/// ```gleam |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these markdown code blocks should be properly closed
/// ## Examples | ||
/// | ||
/// ```gleam | ||
/// to_string_fixed(2.3, 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function name in the examples here doesn't match the code.
Is this approach able to handle exponential notation?
float.to_string(0.000000000144) == "1.44e-10" The unit tests don't currently cover this case AFAICT. |
Closing due to inactivity. Please feel free to reopen! Thank you |
Issue
Changes
Notes
Functionality works, but I feel like my solution is a lot of code. I was fighting circle dependency issues and had to use probably too many externals. I am looking forward to this review. First time contributing to open source and only week using gleam so bare with me.