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

Extract a separate unfiltered_rows_buffer module. #527

Merged
merged 2 commits into from
Nov 11, 2024

Conversation

anforowicz
Copy link
Contributor

Just a minor refactoring to simplify mod.rs and push some of the complexity into a separate, new unfiltered_rows_buffer module.

@anforowicz anforowicz force-pushed the unfiltered-rows-buffer branch from c5097ce to 7183d21 Compare October 30, 2024 23:05
@fintelia
Copy link
Contributor

fintelia commented Oct 31, 2024

The code itself looks good to me, but I'm not thrilled with the naming. The buffer generally holds both filtered and unfiltered data at the same time and while it is true that the current and previous rows are present, there also might be tens/hundreds of kilobytes of data beyond that which doesn't correspond to a specific number of rows. I'm not quite sure what a better name might be

@anforowicz
Copy link
Contributor Author

The code itself looks good to me, but I'm not thrilled with the naming. The buffer generally holds both filtered and unfiltered data at the same time and while it is true that the current and previous rows are present, there also might be tens/hundreds of kilobytes of data beyond that which doesn't correspond to a specific number of rows. I'm not quite sure what a better name might be

Fair feedback :-). RawImageData? RawScanlines? UnfilteringBuffer?

@HeroicKatora
Copy link
Member

I like UnfilteringBuffer, it clearly points to the responsibilities and its use.

@anforowicz anforowicz force-pushed the unfiltered-rows-buffer branch from 7183d21 to 70a217d Compare November 1, 2024 17:49
@HeroicKatora HeroicKatora merged commit a31e67a into image-rs:master Nov 11, 2024
19 checks passed
@HeroicKatora
Copy link
Member

Thank you, both code and naming fit well in the existing code flow 👍

@anforowicz anforowicz deleted the unfiltered-rows-buffer branch November 14, 2024 15:14
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