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

Add pad_tail_using and interleave_shortest adaptors. #31

Merged
merged 2 commits into from
Jun 5, 2015

Conversation

DanielKeep
Copy link
Contributor

pad_tail_using ensures an iterator has some minimum number of elements, with a closure used to fill in the missing ones.

interleave_shortest does exactly what it says on the tin.

Both are adapted from grabbag.


#[inline]
fn next(&mut self) -> Option<I::Item> {
match self.phase {
Copy link
Member

Choose a reason for hiding this comment

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

I think we can shorten this with phase = !phase here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can, but I really didn't want to do that because then you need to match on !phase, which is weird. If you want to cut the repetition out, I suppose I can bind the result of the match to a local r, then flip phase after the match, then return r... but the code would be the same length anyway.

I dunno; might just be my penchant for state machines. :P

@bluss
Copy link
Member

bluss commented Jun 4, 2015

Looking forward to these prs from grabbag! Don't you think a name like pad_to or pad_to_length would be better?

@DanielKeep
Copy link
Contributor Author

@bluss From the commit message:

I've chosen the using suffix as I believe possibly useful variants of
this would better suit the alternatives:

  • to: best for a variant that just repeats the final element of the base sequence.
  • from/with: sounds like it should take another iterator as input; i.e. a kind of chain+take combination.

I hadn't considered length, but I think that belongs more with "repeats final element".

@bluss
Copy link
Member

bluss commented Jun 4, 2015

That's a good point. I didn't look at the commit msg, away from my computer today.. What about simply pad_using?

I've chosen the `using` suffix as I believe possibly useful variants of
this would better suit the alternatives:

* `to`: best for a variant that just repeats the final element of the base
  sequence.
* `from`/`with`: sounds like it should take another iterator as input;
  *i.e.* a kind of `chain`+`take` combination.
@DanielKeep
Copy link
Contributor Author

Ok, I think that did it. Not sure if you get a notification for that or not.

@bluss
Copy link
Member

bluss commented Jun 4, 2015

Leaving a message here is the best yeah, no notification on just pushes.

@bluss
Copy link
Member

bluss commented Jun 4, 2015

Looks fine. I'm a stubborn mind, so I wanted to ask. Is it ok if I merge this? I'm going to change the name to what I prefer, is that ok?

@DanielKeep
Copy link
Contributor Author

Yeah, if you want to change the names, go ahead; it's your prerogative, after all.

Disclaimer: I'm not going to be held responsible for naming conflicts with future hypothetical pad_to_* variants. :P

@bluss bluss merged commit b9386df into rust-itertools:master Jun 5, 2015
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.

2 participants