Skip to content
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

feat(NODE-6086): add Double.fromString() method #671

Merged
merged 6 commits into from
Apr 18, 2024

Conversation

aditi-khare-mongoDB
Copy link
Contributor

@aditi-khare-mongoDB aditi-khare-mongoDB commented Apr 16, 2024

Description

Add static Double.fromString() method.

What is changing?

Is there new documentation needed for these changes?

Yes, there are new API docs.

What is the motivation for this change?

NODE-3660 user ticket, our validation in the Double is lacking. In V7, we will add Double.fromString(value) validation call into the Double constructor's string case.

Release Highlight

Add static Double.fromString method

This method attempts to create an Double type from a string, and will throw a BSONError on any string input that is not representable as a IEEE-754 64-bit double.
Notably, this method will also throw on the following string formats:

  • Strings in non-decimal and non-exponential formats (binary, hex, or octal digits)
  • Strings with characters other than sign, numeric, floating point, or slash characters (Note: 'Infinity', '-Infinity', and 'NaN' input strings are still allowed)
  • Strings with leading and/or trailing whitespace
    Strings with leading zeros, however, are also allowed.

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@aditi-khare-mongoDB aditi-khare-mongoDB changed the title feat(NODE-6808): Add Double.fromString() method feat(NODE-6086): Add Double.fromString() method Apr 16, 2024
@aditi-khare-mongoDB aditi-khare-mongoDB changed the title feat(NODE-6086): Add Double.fromString() method feat(NODE-6086): add Double.fromString() method Apr 16, 2024
@aditi-khare-mongoDB aditi-khare-mongoDB marked this pull request as ready for review April 17, 2024 14:23
@baileympearson baileympearson self-assigned this Apr 17, 2024
@baileympearson baileympearson added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Apr 17, 2024
*
* @param value - the string we want to represent as an double.
*/
static fromString(value: string): Double {
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment here as on the Int32.fromString() PR: can we throw descriptive errors for each case we're checking for in the if statement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, on it


for (const [testName, value, expectedDouble] of acceptedInputs) {
context(`when case is ${testName}`, () => {
it(`should return Double that matches expected value`, () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment here about test titles.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just changed them, going to push after my tests run!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Primary Review In Review with primary reviewer, not yet ready for team's eyes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants