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: Impl Iterator::size_hint() for Dictionary (Closes: #560). #583

Merged
merged 1 commit into from
Jan 31, 2024

Conversation

tuto193
Copy link
Contributor

@tuto193 tuto193 commented Jan 29, 2024

The following Dictionary Iters now implement Iterator::size_hint():

  • DictionaryIter (base for all others)
  • Iter<'a>
  • Keys<'a>
  • TypedIter<'a, K, V>
  • TypedKeys<'a, K>

@GodotRust
Copy link

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-583

Copy link
Member

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

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

Thank you, also for the tests! 🙂

@Bromeon Bromeon added quality-of-life No new functionality, but improves ergonomics/internals c: core Core components labels Jan 29, 2024
@tuto193 tuto193 force-pushed the 560_size_hint_for_dict branch from 68195ad to 693a58a Compare January 29, 2024 23:37
@tuto193 tuto193 requested a review from Bromeon January 29, 2024 23:38
@Bromeon
Copy link
Member

Bromeon commented Jan 29, 2024

An issue with indexes is that it's possible (although unlikely) that an element is added/removed while iterating (through a cloned dictionary). This problem also affects the size though.

size_hint doesn't have to be correct, which would allow such edge cases.

Maybe you can see if this can be reproduced in a separate itest? Clone dictionary, start iterating, add/remove key in the copy...

@tuto193 tuto193 force-pushed the 560_size_hint_for_dict branch from 693a58a to 4b513e4 Compare January 30, 2024 23:09
@tuto193
Copy link
Contributor Author

tuto193 commented Jan 30, 2024

An issue with indexes is that it's possible (although unlikely) that an element is added/removed while iterating (through a cloned dictionary). This problem also affects the size though.

size_hint doesn't have to be correct, which would allow such edge cases.

Maybe you can see if this can be reproduced in a separate itest? Clone dictionary, start iterating, add/remove key in the copy...

The test is a good way to see that size_hint() isn't always correct. Let me know, if this suffices! :)

@tuto193 tuto193 force-pushed the 560_size_hint_for_dict branch 2 times, most recently from 1fff44c to 73ca8d5 Compare January 31, 2024 17:11
@tuto193 tuto193 requested a review from Bromeon January 31, 2024 17:12
Copy link
Member

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

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

Thanks a lot! One small thing remaining 🙂

…st#560).

The following `Dictionary` `Iters` now implement `Iterator::size_hint()`:
- `DictionaryIter` (base for all others)
- `Iter<'a>`
- `Keys<'a>`
- `TypedIter<'a, K, V>`
- `TypedKeys<'a, K>`
@tuto193 tuto193 force-pushed the 560_size_hint_for_dict branch from 73ca8d5 to c16bb6c Compare January 31, 2024 17:58
@tuto193 tuto193 requested a review from Bromeon January 31, 2024 17:59
Copy link
Member

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@Bromeon Bromeon enabled auto-merge January 31, 2024 18:12
@Bromeon Bromeon added this pull request to the merge queue Jan 31, 2024
Merged via the queue into godot-rust:master with commit 76f9d72 Jan 31, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: core Core components quality-of-life No new functionality, but improves ergonomics/internals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants