8000 feat: add size_hint for Iter::collect by illusory0x0 · Pull Request #2092 · moonbitlang/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat: add size_hint for Iter::collect #2092

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat: add size_hint for Iter::collect #2092

wants to merge 1 commit into from

Conversation

illusory0x0
Copy link
Contributor

The internal iterator doesn't know the number of elements, and it makes sense to add size_hint as a default parameter

@illusory0x0
feat: add size_hint for Iter::collect
b28485f 
The internal iterator doesn't know the number of elements, and it makes sense to add size_hint as a default parameter
@peter-jerry-ye-code-review peter-jerry-ye code review
Copy link
Initial array capacity optimization using size_hint

Category
Performance
Code Snippet
pub fn Iter::to_array[T](self : Iter[T], size_hint~ : Int = 0) -> Array[T] {
let result = Array::new(capacity=size_hint)
Recommendation
Consider documenting the performance implications of size_hint in the function comment. Also consider validating that size_hint is non-negative.
Reasoning
Pre-allocating array capacity can significantly reduce memory reallocations during collection. Users should understand this optimization opportunity through documentation.

Code duplication eliminated between collect and to_array

Category
Maintainability
Code Snippet
pub fn Iter::collect[T](self : Iter[T], size_hint~ : Int = 0) -> Array[T] {
self.to_array(size_hint~)
}
Recommendation
Consider adding a note in the collect documentation that it delegates to to_array for implementation details
Reasoning
The change reduces code duplication which is good for maintenance, but documentation should make the relationship between these methods clear to users

Default value of size_hint parameter uses '..' syntax which needs clarification

Category
Correctness
Code Snippet
collect[T](Self[T], size_hint~ : Int = ..) -> Array[T]
Recommendation
Either document the meaning of '..' for default value or consider using explicit '0' in the interface definition to match implementation
Reasoning
The interface uses '..' while implementation uses '0' as default. This inconsistency could be confusing and should be aligned or explained

@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 6682

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.002%) to 92.451%
Totals Coverage Status
Change from base Build 6679: -0.002%
Covered Lines: 6136
Relevant Lines: 6637
💛 - Coveralls

@peter-jerry-ye
Copy link
Collaborator

You can have:

let arr = Array::new(capacity=xxx)
iter.each(arr.push(_))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@illusory0x0 @coveralls @peter-jerry-ye
0