with_position's/Position's API improvable?

[phimuemue]

I just tried to use with_position, and realized that its Item is an enum wrapping the actual item:

pub enum Position<T> {
    First(T),
    Middle(T),
    Last(T),
    Only(T),
}

This looks ok at first glance, but it apparently impedes manipulating the wrapped value and keeping the information whether it's first, middle, last, or only: It seems you must unpack the value (via into_inner) and manually construct e.g. a Position<()> - which feels very inconvenient.

I hope I did not miss any obvious solutions (please tell me if I did), so here's two alternatives how Itertools could overcome this:

1. Introduce Position::map that allows transforming the wrapped value (akin to Option::map).
   • Pro: Avoids API breakage.
   • Con: Seems to be a workaround./Does possibly not cover all use cases. (E.g: What if you want to clone the wrapped value and keep the original (i.e. go from Position<T> to (Position<T>, T))?)
2. Remove T from Position and make the Item=(Position, T) so that the position itself and the wrapped value can be used independently.
   • Pro: Seems to be cleaner and more flexible.
   • Con: API breakage. (Could be mitigated by introducing another function, e.g with_position_2).

Personally, I'd vote for option 2, accepting the API breakage.

@jswrenn @scottmcm Should we change this?

[scottmcm]

with_position_2 would make me sad.

Maybe an inherent method for Position<T> -> (Position<()>, T)? So you could always do .with_position().map(Position::into_parts) or similar for the cases where the tuple is more convenient?

I'm kinda inspired by https://docs.rs/itertools/latest/itertools/enum.MinMaxResult.html#method.into_option for this, as another method for "well, sometimes you'd rather work with it in a different form".

Could even go a step further and change Position<T> to Position<T = ()> to encourage the used-without-payload case without it being a breaking change? Dunno if that's a good idea...

[jswrenn]

I like your second possibility. I don't think there's any need for a with_position_2. We can do a major release for this.

We'll need to do one anyways to fix the intersperse collision.

[Philippe-Cholet]

I think this should be closed, thanks to #699.

[jswrenn]

Thanks!