Unify convenience map functions #464
Conversation
|
|
||
| #[derive(Clone)] | ||
| #[must_use = "iterator adaptors are lazy and do nothing unless consumed"] | ||
| pub struct MapSpecialCase<I, F> { |
There was a problem hiding this comment.
| pub struct MapSpecialCase<I, F> { | |
| pub struct MapSpecialCase<I, F: ?Sized> { |
This way unsized “special functions” can be used, including trait objects.
There was a problem hiding this comment.
Good observation! Are unsized functions common? Alternatively, we could reverse the order of fields in MapSpecialCase and make I: ?Sized, thereby supporting iterator trait objects. (Unfortunately, it's one or the other.)
There was a problem hiding this comment.
Thanks for this nice idea. In general, I am leaning to incorporate changes that generalize functionality.
However, I'd like to postpone this one and - if we decide to accept ?Sized functions - do it uniformly throughout itertools. I think my changes did not change behavior in this regard, so I hope postponing is ok.
@pthariensflame As I'm no expert in this area: Could you showcase a situation that requires F: ?Sized?
There was a problem hiding this comment.
Sure! If we have an unsized trait object dyn Fn(X) -> Y, then we get an unsized MapSpecialCaseFnOk<dyn Fn(X) -> Y>, which is then your example of an unsized F.
There was a problem hiding this comment.
However, I'd like to postpone this one and - if we decide to accept ?Sized functions - do it uniformly throughout itertools. I think my changes did not change behavior in this regard, so I hope postponing is ok.
Fine by me!
There was a problem hiding this comment.
@pthariensflame I'm curious about the actual use-case dyn Fn might appear in. I've never needed to dynamically dispatched function objects before.
There was a problem hiding this comment.
Yeah, I'm fine with waiting too. This isn't essential functionality, just mildly helpful at best.
As far as dyn Fn, I can't point to any specific code, but it might come up when abstracting over dynamically swappable actions. For example: imagine a pipeline of scriptable blocks that the user can specify in a GUI, and it gets compiled down to functions that invoke the scripts and munge the values flowing through as necessary. The whole pipeline is then composed and boxed up as a dyn Fn so that it can be altered on the fly as the application runs.
| impl<I, R> Iterator for MapSpecialCase<I, R> | ||
| where | ||
| I: Iterator, | ||
| R: MapSpecialCaseFn<I::Item>, |
There was a problem hiding this comment.
| R: MapSpecialCaseFn<I::Item>, | |
| R: ?Sized + MapSpecialCaseFn<I::Item>, |
| impl<I, R> ExactSizeIterator for MapSpecialCase<I, R> | ||
| where | ||
| I: ExactSizeIterator, | ||
| R: MapSpecialCaseFn<I::Item>, |
There was a problem hiding this comment.
| R: MapSpecialCaseFn<I::Item>, | |
| R: ?Sized + MapSpecialCaseFn<I::Item>, |
| impl<I, R> DoubleEndedIterator for MapSpecialCase<I, R> | ||
| where | ||
| I: DoubleEndedIterator, | ||
| R: MapSpecialCaseFn<I::Item>, |
There was a problem hiding this comment.
| R: MapSpecialCaseFn<I::Item>, | |
| R: ?Sized + MapSpecialCaseFn<I::Item>, |
| } | ||
|
|
||
| #[derive(Clone)] | ||
| pub struct MapSpecialCaseFnOk<F>(F); |
There was a problem hiding this comment.
| pub struct MapSpecialCaseFnOk<F>(F); | |
| pub struct MapSpecialCaseFnOk<F: ?Sized>(F); |
|
|
||
| impl<F, T, U, E> MapSpecialCaseFn<Result<T, E>> for MapSpecialCaseFnOk<F> | ||
| where | ||
| F: FnMut(T) -> U, |
There was a problem hiding this comment.
| F: FnMut(T) -> U, | |
| F: ?Sized + FnMut(T) -> U, |
| pub fn map_ok<I, F, T, U, E>(iter: I, f: F) -> MapOk<I, F> | ||
| where | ||
| I: Iterator<Item = Result<T, E>>, | ||
| F: FnMut(T) -> U, |
There was a problem hiding this comment.
| F: FnMut(T) -> U, | |
| F: ?Sized + FnMut(T) -> U, |
… import used items
|
This looks good to me! bors r+ |
|
Build succeeded: |
3 participants
In response to #283: I think we should employ a uniform architecture for our
map-specializations (as they already diverged). This PR is meant to unify the implementations so we essentially only need to parametrize the mapping function:MapIntotoMapSpecialCase(which can be used as basis for all our map convenience functions) and defineMapIntoin terms ofMapSpecialCasecollect(was specialized onMapOk, so I guess we want to keep it for now)MapOkin terms ofMapSpecialCasesize_hint,fold,collectmap_xyzare conflicting anyway, move them to an own module (similar tocoalesce)