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

doc: update List.mo #616

Merged
merged 3 commits into from
Feb 21, 2024
Merged

doc: update List.mo #616

merged 3 commits into from
Feb 21, 2024

Conversation

crusso
Copy link
Contributor

@crusso crusso commented Feb 12, 2024

Fix bugs is doc comments

Fix bugs is doc comments
src/List.mo Show resolved Hide resolved
@crusso crusso requested a review from ggreif February 12, 2024 12:47
@@ -169,7 +169,9 @@ module {
///
/// Runtime: O(size)
///
/// Space: O(1)
/// Space: O(size)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the linear size originate from any (internal) temporary allocations during the iterative iteration?

Copy link
Contributor Author

@crusso crusso Feb 20, 2024

Choose a reason for hiding this comment

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

Yeah, I'm not really sure how to document this. Should it just be 0(1)? If the function allocates non-zero constant space, then I think iterating it should allocate linear space (if the space is retained).

Copy link
Member

Choose a reason for hiding this comment

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

What about O(space(f))?

(was just passing by...)

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.

Thanks for the improvement. This looks very good.
I only have a question regarding the documented size of iterate().

@ggreif ggreif changed the title doc: update List.mo doc: update List.mo Feb 12, 2024
@crusso crusso enabled auto-merge (squash) February 21, 2024 17:27
@crusso crusso merged commit c86d76f into master Feb 21, 2024
6 checks passed
@crusso crusso deleted the crusso-patch-2 branch February 21, 2024 17:30
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.

4 participants