Add `get_mut`
Closed this issue · 6 comments
I'm not sure why wasn't fn get_mut(&mut self) -> Option<&mut Self::Item>;
added already, but it would be great to have it if possible.
Not all streaming iterators can support that -- for example ConvertRef
or MapRef
don't have any mutable access. Maybe it could be in a new trait for compatible iterators to implement.
I'm looking into implementing this and have tried two approaches:
- As explained here, have two different traits with the exact same functionality but returning a mutable reference
- Add a trait that extends StreamingIterator and adds a
get_mut
,next_mut
andmap_deref_mut
(perhaps others?)
In both cases add a ConvertMut
and convert_mut
function
I'm inclined to think that the second solution makes more sense: For most functions all it's really needed is a reference and that way a type can implement both traits easily without having to reimplement advance:
/// An interface for dealing with mutable streaming iterators.
pub trait StreamingIteratorMut: StreamingIterator {
/// Returns a mutable reference to the current element of the iterator.
///
/// The behavior of calling this method before `advance` has been called is unspecified.
fn get_mut(&mut self) -> Option<&mut Self::Item>;
/// Advances the iterator and returns the next value as a mutable reference.
///
/// The behavior of calling this method after the end of the iterator has been reached is
/// unspecified.
///
/// The default implementation simply calls `advance` followed by `get_mut`.
#[inline]
fn next_mut(&mut self) -> Option<&mut Self::Item> {
self.advance();
(*self).get_mut()
}
/// Creates a regular, non-streaming iterator which transforms elements of this iterator by
/// passing them mutably to a closure.
#[inline]
fn map_deref_mut<B, F>(self, f: F) -> MapDerefMut<Self, F>
where
Self: Sized,
F: FnMut(&mut Self::Item) -> B,
{
MapDerefMut { it: self, f }
}
}
The main disadvantage is that the mutable version of some functions that might be desirable like for example map_mut
can't be implemented this way as MapMut
would have to implement StreamingIterator
as well but can't cause get
returns a non mutable reference but the MatMut
function expects a mutable one:
/// A streaming iterator which transforms the elements of a streaming iterator.
#[derive(Debug)]
pub struct MapMut<I, F> {
it: I,
f: F,
}
impl<I, B: ?Sized, F> StreamingIterator for MapMut<I, F>
where
I: StreamingIteratorMut,
F: Fn(&mut I::Item) -> &mut B,
{
type Item = B;
#[inline]
fn get(&self) -> Option<&B> {
let it = &self.it;
let f = &self.f;
// this is an error cause it is behind a non mutable borrow
// it.get_mut().map(|current| (f)(current) as &B)
// this is also an error cause get returns a non mutable reference but F expects a mutable one
it.get().map(|current| (f)(current) as &B)
}
}
``
How serendipitous, I was also just working on this. I went with your second style, an extension trait adding get_mut
, next_mut
, fold_mut
, and for_each_mut
, and another trait for double-ended next_back_mut
and rfold_mut
. Here's a comparison on top of my #22 changes:
cuviper/streaming-iterator@sources...cuviper:mutable
I hesitate to make too many new _mut
methods, especially where they need new types, but map_deref_mut
might be a good one indeed. I did add convert_mut
and ConvertMut
in mine too.
That's great!, yes I think most methods can do just fine with a non mutable reference. And yes I also added the mutable DoubleEnded trait but forgot to post it.
Here's the implementation for MapDerefMut
although it's pretty straightforward:
/// A regular, non-streaming iterator which transforms the elements of a streaming iterator.
#[derive(Debug)]
pub struct MapDerefMut<I, F> {
it: I,
f: F,
}
impl<I, B, F> Iterator for MapDerefMut<I, F>
where
I: StreamingIteratorMut,
F: FnMut(&mut I::Item) -> B,
{
type Item = B;
#[inline]
fn next(&mut self) -> Option<Self::Item> {
self.it.next_mut().map(&mut self.f)
}
#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
self.it.size_hint()
}
}
impl<I, B, F> DoubleEndedIterator for MapDerefMut<I, F>
where
I: DoubleEndedStreamingIteratorMut,
F: FnMut(&mut I::Item) -> B,
{
#[inline]
fn next_back(&mut self) -> Option<Self::Item> {
self.it.next_back_mut().map(&mut self.f)
}
}
We should try to at least share credit, since we already duplicated effort. 🙂
Would it work for you to send that MapDerefMut
as a PR adding to my branch? Or we could do that as consecutive PRs here. I would also add a fold
to your Iterator
, which should be simple with self.it.fold_mut(...)
.
Nah no worries, feel free to use the code i posted here, you've contributed already to this project so it'll surely be faster for you to get your PR merged.
I also tried a fold_mut in the second version with separated traits for mut and not mut and I see how that can be useful if only for specializations of the Mutable iterators.