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

[Breaking change] rename more() to something more explicit. #127

Open
Carreau opened this issue Aug 26, 2023 · 2 comments
Open

[Breaking change] rename more() to something more explicit. #127

Carreau opened this issue Aug 26, 2023 · 2 comments

Comments

@Carreau
Copy link
Contributor

Carreau commented Aug 26, 2023

See #124 and #126, the name more(), is confusing as it indicate that we want to keep iterating over buckets/records whatever even if they all will be empty. Maybe rename to something like more_with_zero_count, or more_empty ? Though this does not convey that all the subsequent one will be empty.

@Carreau
Copy link
Contributor Author

Carreau commented Aug 26, 2023

I'm also unsure if we can do it in a smooth backward compatible manner?

pub trait PickyIterator<T: Counter> {
    ...

    // call more if defined (backward compatibility), 
    #[allow(deprecated)]
    fn more_with_zero_count(&mut self, index_to_pick: usize) -> bool {
        self.more(index_to_pick)
    }

    // provide a default impl for implementation that have already renamed more to
    // more_with_zero_count.
    #[deprecated(...)]
    fn more(&mut self, index_to_pick: usize) -> bool {
        self.more_with_zero_count(index_to_pick)
    }
}

It's a bit dumb as the default impl obviously have infinite recursion, but this make it easy for any implementer of PickyIterator to be supported both with the old and the new name until the old name get removed.

@Carreau Carreau changed the title rename more() to something more explicit. [Breaking change] rename more() to something more explicit. Aug 26, 2023
@jonhoo
Copy link
Collaborator

jonhoo commented Sep 9, 2023

I'm just going to leave this open until the next time we do a breaking release :)

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

No branches or pull requests

2 participants