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

Nat documentation and fromText() utility #517

Merged
merged 6 commits into from
Feb 6, 2023
Merged

Conversation

kentosugama
Copy link
Contributor

No description provided.

@kentosugama kentosugama marked this pull request as ready for review February 2, 2023 23:27
@kentosugama kentosugama changed the title Nat documentation Nat documentation and fromText() utility Feb 2, 2023
@kentosugama kentosugama changed the title Nat documentation and fromText() utility Nat documentation and fromText() utility Feb 2, 2023
src/Nat.mo Outdated
@@ -1,72 +1,299 @@
/// Natural numbers
/// Utility functions for working with natural numbers.
Copy link
Contributor

@luc-blaeser luc-blaeser Feb 3, 2023

Choose a reason for hiding this comment

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

Update: (Just saw that it is mentioned below). But maybe it is useful to mention it also in the header that the numbers have infinite precision, in symmetry with the documentation Int.mo.

src/Nat.mo Outdated

module {

/// Infinite precision natural numbers.
public type Nat = Prim.Types.Nat;

/// Conversion.
public let toText : Nat -> Text = Int.toText;
/// Converts a natural number to its textual representation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Detail (unsure whether worth mentioning): Without underscore formatting for thousand digit separators.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will add

/// ```motoko include=import
/// Nat.fromText "1234" // => ?1234
/// ```
public func fromText(text : Text) : ?Nat {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just noticed that we do not have this function in Int. Maybe it would be useful to offer it there two (and even combine the implementation, similar to toText).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#520 Yeah that sounds good!

/// to the existing `==` operator) is so that you can use it as a function
/// value to pass to a higher order function. It is not possible to use `==`
/// as a function value at the moment.
///
Copy link
Contributor

@luc-blaeser luc-blaeser Feb 3, 2023

Choose a reason for hiding this comment

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

Great comment. Maybe we should also improve Int with this explanation.

/// let buffer1 = Buffer.Buffer<Nat>(3);
/// let buffer2 = Buffer.Buffer<Nat>(3);
/// Buffer.equal(buffer1, buffer2, Nat.equal) // => true
/// ```
Copy link
Contributor

Choose a reason for hiding this comment

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

And nice example demonstrating where it makes really sense to use the equal function.

public func compare(x : Nat, y : Nat) : { #less; #equal; #greater } {
if (x < y) { #less } else if (x == y) { #equal } else { #greater }
};

/// Returns the sum of `x` and `y`, `x + y`.
///
/// Example:
Copy link
Contributor

@luc-blaeser luc-blaeser Feb 3, 2023

Choose a reason for hiding this comment

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

Maybe it could help to mention (again): No overflow since Nat has infinite precision. Same for mul().

/// ```motoko include=import
/// import Array "mo:base/Array";
/// Array.foldLeft([2, 3, 1], 1, Nat.mul) // => 6
/// ```
public func mul(x : Nat, y : Nat) : Nat { x * y };

/// Returns the division of `x` by `y`, `x / y`.
/// Traps when `y` is zero.
Copy link
Contributor

@luc-blaeser luc-blaeser Feb 3, 2023

Choose a reason for hiding this comment

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

Suggested change
/// Traps when `y` is zero.
/// Returns the unsigned integer division of `x` by `y`, `x / y`.
/// Rounds the quotient towards zero, which is the same as truncating the decimal places of the quotient.

/// Note: The reason why this function is defined in this library (in addition
/// to the existing `/` operator) is so that you can use it as a function
/// value to pass to a higher order function. It is not possible to use `/`
/// as a function value at the moment.
public func div(x : Nat, y : Nat) : Nat { x / y };

/// Returns the remainder of `x` divided by `y`, `x % y`.
/// Traps when `y` is zero.
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to div, the remainder of the unsigned integer division.

public func rem(x : Nat, y : Nat) : Nat { x % y };

/// Returns `x` to the power of `y`, `x ** y`.
///
Copy link
Contributor

@luc-blaeser luc-blaeser Feb 3, 2023

Choose a reason for hiding this comment

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

Suggested change
///
/// Traps when `y > 2 ** 32`.
/// No overflow since `Nat` has infinite precision.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh! Didn't know that

Copy link
Contributor

@luc-blaeser luc-blaeser left a comment

Choose a reason for hiding this comment

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

Great documentation. And source for further improving also Int. Ideally, we could document both symmetrically.

@kentosugama
Copy link
Contributor Author

@luc-blaeser Thanks for the feedback as always

@kentosugama kentosugama merged commit 11e160a into master Feb 6, 2023
@kentosugama kentosugama deleted the nat-documentation branch February 6, 2023 17:43
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