My first pull request for rust has been merged πŸŽ‰

Β· Web Β· 2 Β· 1 Β· 5

Hei wow, that is so cool! Congratulations.

@adi Congrats! You mentioned that you were stuck with the next `nth_back` implementation, I'd love to help out if you're still interested. (I recently also had my first Rust PRs merged and they were related to iterators as well)

@timvermeulen Thanks 😊 I'm stuck with a design decision and I don't want to implement just something without proper integration. I wrote down my thoughts in this commit message maybe you can give me some input there?
The next steps for me will be editing the ticket and insert a todo list of implementations (I already have it available) and then continue with an implementation of an easier trait.

@adi Hmm, looks like the Step trait is in a weird place right now (looking at – maybe it's not worth it implementing nth_back for Range at this stage :) But I agree that it probably makes sense for Step to have sub_usize rather than some other trait.

@timvermeulen Thank you for the investigation, I agree, we'd better skip the Range trait for now.

Yesterday I updated my original issue and added a task list. Feel free to implement some of those specializations:

@adi Thanks, that's a great list! I'll be sure to comment in that thread if/when I implement any, so you can cross them off.

@adi I've been thinking about `nth` and `nth_back` recently, and it turns out that iterators like `Chain` and `Flatten` can't properly forward them to their inner iterators because the `None` case doesn't indicate what's left of `n`.

In order to allow this kind of forwarding, `Iterator` would need two extra methods `try_nth` and `try_nth_back` that both return `Result<Self::Item, usize>`... Seems not very likely to be accepted.

Thoughts? Good idea, or not worth it?

@timvermeulen Hi, after looking the existing PR for Chain, that someone else did, I see what you mean.

I like your idea of try_nth, but I don't know if solving these edge cases is worth the effort. Also it is not guaranteed that the length of an iterator fits into usize.

Except for iterators implementing ExactSizeIterator, but for those we can make an even more specific implementation for Chain anyway. Similar for Flatten.

Err... I made an error in my last post. While the length of an iterator generally is not guaranteed to fit in usize, it is guaranteed for your try_nth, where the argument is usize and the failing result is always a smaller integer. πŸ˜ƒ

@adi While that's true, I'm still not sure how to correctly implement `try_nth` for `Range` because of all the overflow checks in the `Step` trait, even though that one is supposed to be the easiest to implement πŸ˜ƒ

@adi I don't know if I'd call it an edge case to be able to call `nth` or `step_by` on chained or flattened iterators without any additional overhead. It's what I would expect from a language that prides itself for its zero-cost abstractions.

What did you mean by "an even more specific implementation"? Isn't `try_nth` all you need for a perfect implementation of `nth` and `try_nth` for `Chain`?

@timvermeulen You're right, the more I think about it the more I believe it is a good thing to have something like `try_nth`.

With the specialization for the `ExactSizeIterator` I was suggesting an alternative solution, but this would not cover all the cases and `try_nth` will handle them all correctly.

@adi To be honest, I mostly want `try_nth` because of how nice that the implementation of `nth` makes for `Chain`. In practice, I think that most relevant iterators will implement `ExactSizeIterator` and will therefore not "need" `try_nth`.

Although now that I think about it, `Chain` itself doesn't implement `ExactSizeIterator` (because of overflow, I assume), and chaining more than two iterators isn't super uncommon. So that's a good argument for `try_nth`.

@adi It's not very pretty, but I think this is a correct implementation for `Range::try_nth`... Overflow is the worst πŸ˜ƒ

Oh, wow, looks good to me. The implementation would get simpler and easier to understand if the error result returned the steps in between instead of the remaining steps, but you should go for the interface which is the most useful to the user, which is probably the one you have now.

I really like the idea of try_nth, which is basically just the Result version of nth's Option 😎. Will you try to get it stabilized?

@adi Many `try_nth` implementations are actually simpler when the error case represents the remaining steps πŸ˜ƒ For instance, the final value of the `n` variable in the current default implementation of `Iterator::nth` is exactly the number of steps remaining. And it makes `Chain::try_nth` also really simple because it can just pass that value from the first iterator to `try_nth` of the second iterator, and then return its return value.

So `Range::try_nth` seems to be an outlier, complexity wise.

@adi I'm definitely planning to submit a PR with an implementation, but it would need proper documentation, and I have a hard time putting into words what the error value represents. It's not exactly the number of elements that the iterator was "missing", since it's zero-indexed.

Saying that it's `n` minus the length of the iterator would be correct, but that doesn't really clarify why the length isn't returned instead... The reason is composition, but it's hard to phrase concisely πŸ™‚

@adi Whoops, I didn't realize that I basically redid all the work of adding Step::sub_usize that you did back in April...

Either way, now that it has landed, you can submit the Range::nth_back implementation that you've been sitting on πŸ˜ƒ

And it looks like nth_back will be stabilized in 1.37, that's really cool.

nth_back being stabilized sounds cool. I will look into the specializations after I come home from vacation and I will still have some days off :-) Thanks for your support.

Sign in to participate in the conversation – a Fediverse instance for & by the Chaos community