Skip to content

Clean up code by using Iterator::collect() when constructing instance tables #2918

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

Merged
merged 6 commits into from
Jul 23, 2025

Conversation

Firestar99
Copy link
Collaborator

Quite a few nodes editing InstanceTables have this sort of code:

let mut result_table = RasterDataTable::default();
for mut image_frame_instance in image_frame.instance_iter() {
  /// ...
  result_table.push(image_frame_instance);
}
result_table

Now you can write them like this:

image_frame
  .instance_iter()
  .map(|mut image_frame_instance| {
    /// ...
    image_frame_instance
  })
  .collect()

Steams also have the advantage that in simple cases like these, the resulting vec can use Iterator:size_hint() to allocate the exact amount of entries it needs, whereas the for loop variant it may go through multiple resizes and overallocate memory.

Copy link
Member

@TrueDoctor TrueDoctor left a comment

Choose a reason for hiding this comment

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

Code looks good @Keavon can comment on this more but I think this should be good to mereg

@Keavon Keavon changed the title Iterator::collect() Instance Tables Clean up code by using Iterator::collect() when constructing instance tables Jul 23, 2025
@Keavon Keavon enabled auto-merge (squash) July 23, 2025 05:47
@Keavon Keavon merged commit 890da6a into master Jul 23, 2025
4 checks passed
@Keavon Keavon deleted the collect_instances branch July 23, 2025 05:51
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.

3 participants