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

Tweak sortcache.SortCache API #52414

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Tweak sortcache.SortCache API #52414

wants to merge 1 commit into from

Conversation

rosstimothy
Copy link
Contributor

  • Adds (SortCache) Clear to allow resetting an existing cache
  • Converts (SortCache) Ascend and (SortCache) Descend to return an iter.Seq[T] instead of taking an iteration function
  • Removes (SortCache) AscendPaginated and (SortCache) DescendPaginated

Leveraging iter.Seq aligns the API with the direction that Go is heading in regards to iteration. Removing of the paginated functions does put the onus on callers to paginate, however, in the only cases these were used there are now fewer allocations. AscendPaginated was unused and DescendPaginated was only used in two places - both of which would benefit from migrating away from using streams in favor of iter.Seq.

The new Clear method makes it easier for callers to reset an existing cache. Without this change the only way to reset a cache would be to replace the entire cache with a new one. This places the burden on callers to apply locking and handle concurrent read/write/deletes and swapping out the cache entirely. That all is eliminated by Clear handling the internal locking to reset the cache state.

- Adds (SortCache) Clear to allow resetting an existing cache
- Converts (SortCache) Ascend and (SortCache) Descend to return
  an iter.Seq[T] instead of taking an iteration function
- Removes (SortCache) AscendPaginated and (SortCache) DescendPaginated

Leveraging iterates aligns the API with the direction that Go is
heading in regards to iteration. Removing of the paginated functions
does put the onus on callers to paginate, however, in the only
cases these were used there are now fewer allocations. AscendPaginated
was unused and DescendPaginated was only used in two places - both
of which would benefit from migrating away from using streams in
favor of iter.Seq.

The new Clear API makes it easier for callers to reset an existing
cache. Without this change the only way to reset a cache would be
to replace the entire cache with a new one. This places the burden
on callers to apply locking and handle concurrent read/write/deletes
and swapping out the cache entirely. That all is eliminated by
Clear handling the internal locking to reset the cache state.
@rosstimothy rosstimothy added the no-changelog Indicates that a PR does not require a changelog entry label Feb 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Indicates that a PR does not require a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant