Skip to content

Instantly share code, notes, and snippets.

@jonhoo
Created April 22, 2020 22:08
Show Gist options
  • Star 31 You must be signed in to star a gist
  • Fork 5 You must be signed in to fork a gist
  • Save jonhoo/2a7fdcf79be03e51a5f95cd326f2a1e8 to your computer and use it in GitHub Desktop.
Save jonhoo/2a7fdcf79be03e51a5f95cd326f2a1e8 to your computer and use it in GitHub Desktop.
#![warn(rust_2018_idioms)]
#[derive(Debug)]
pub struct StrSplit<'haystack, D> {
remainder: Option<&'haystack str>,
delimiter: D,
}
impl<'haystack, D> StrSplit<'haystack, D> {
pub fn new(haystack: &'haystack str, delimiter: D) -> Self {
Self {
remainder: Some(haystack),
delimiter,
}
}
}
pub trait Delimiter {
fn find_next(&self, s: &str) -> Option<(usize, usize)>;
}
impl<'haystack, D> Iterator for StrSplit<'haystack, D>
where
D: Delimiter,
{
type Item = &'haystack str;
fn next(&mut self) -> Option<Self::Item> {
let remainder = self.remainder.as_mut()?;
if let Some((delim_start, delim_end)) = self.delimiter.find_next(remainder) {
let until_delimiter = &remainder[..delim_start];
*remainder = &remainder[delim_end..];
Some(until_delimiter)
} else {
self.remainder.take()
}
}
}
impl Delimiter for &str {
fn find_next(&self, s: &str) -> Option<(usize, usize)> {
s.find(self).map(|start| (start, start + self.len()))
}
}
impl Delimiter for char {
fn find_next(&self, s: &str) -> Option<(usize, usize)> {
s.char_indices()
.find(|(_, c)| c == self)
.map(|(start, _)| (start, start + self.len_utf8()))
}
}
pub fn until_char(s: &str, c: char) -> &'_ str {
StrSplit::new(s, c)
.next()
.expect("StrSplit always gives at least one result")
}
#[test]
fn until_char_test() {
assert_eq!(until_char("hello world", 'o'), "hell");
}
#[test]
fn it_works() {
let haystack = "a b c d e";
let letters: Vec<_> = StrSplit::new(haystack, " ").collect();
assert_eq!(letters, vec!["a", "b", "c", "d", "e"]);
}
#[test]
fn tail() {
let haystack = "a b c d ";
let letters: Vec<_> = StrSplit::new(haystack, " ").collect();
assert_eq!(letters, vec!["a", "b", "c", "d", ""]);
}
@tomasol
Copy link

tomasol commented May 12, 2020

Sorry if this was already asked elsewhere, but I am still not sure why:

if let Some(ref mut remainder) = self.remainder { // (1) ok 
let ref mut remainder = self.remainder?; // (2) nok

if let Some(remainder) = &mut self.remainder { // (3) ok
let remainder = &mut self.remainder?; // (4) nok

if let Some(remainder) = self.remainder.as_mut() { // ok
let remainder = self.remainder.as_mut()?; // ok

I see that if a type is Copy, it gets copied instead of moved. But aren't we dealing with same types in 1 vs 2, 3 vs 4?

@jonhoo
Copy link
Author

jonhoo commented May 13, 2020

The ? in 2 and 4 copies the entire Option when the inner type is Copy. What we then take a mutable reference to is what is inside of that copy, not the original Option. In the last case, we turn Option<T> into Option<&mut T>, which, when copied, still yields a mutable reference into the original Option. Does that help?

@tomasol
Copy link

tomasol commented May 17, 2020

yes, thanks. I wish there was a way to turn off or at least warn about implicit switch from move to copy semantics. Is there a way to guard against this kind of bugs?

@jonhoo
Copy link
Author

jonhoo commented May 18, 2020

Hmm, it's a good question. I wonder if maybe clippy could warn about this somehow. In theory the information may be available to determine it statically.

@iiian
Copy link

iiian commented Apr 3, 2021

Hey @jonhoo, how common is it to see or do:

impl Trait for &Type {...} //?

// such as
impl Delimiter for &str {...}

Additionally, if you intend to use both Type and &Type as implementers of Trait, for instance:

let x1: Vec<_> = StrSplit::new(haystack, MyOtherDelimiterFlavor::new(...)).collect();
let x2: Vec<_> = StrSplit::new(haystack, &MyOtherDelimiterFlavor::new(...)).collect(); 

would one then need to write redundant impl blocks, or is there some syntactic sugar for controlling this?

Thanks

@jonhoo
Copy link
Author

jonhoo commented Apr 4, 2021

It's actually fairly common, precisely to improve the ergonomics of using the trait as you indicate. In general, if you implement a trait that only takes &self it's pretty reasonable to implement the trait for &MyType, &mut MyType and Box<MyType>. If a trait method takes &mut self, skip &MyType. But of course, the downside is that now if the trait changes in the future, more breakage will ensure since your consumers expected to be able to transparently use & (or &mut).

@1717chen
Copy link

1717chen commented Mar 2, 2024

Hi, curious how move and ownership works in the following line

s.find(self).map(|start| (start, start + self.len()))

I suppose the start need to be mutable? So, did we just move ownership of start?

https://gist.github.com/jonhoo/2a7fdcf79be03e51a5f95cd326f2a1e8#file-strsplit-rs-L41

@jonhoo
Copy link
Author

jonhoo commented Mar 2, 2024

No, start doesn't need to be mutable here as we never actually mutate it. Keep in mind that usize is Copy, so start + self.len() copies start and computes a new usize that is then returned.

@1717chen
Copy link

1717chen commented Mar 4, 2024

thanks, as a rust beginner, is something confusing to spot whether a type implemnent copy trait or not. Thanks for making the one the best rust content on Youtube!!

@MaxAsif
Copy link

MaxAsif commented Apr 4, 2024

Hi @jonhoo,

What is the difference between these two approach in next method

    fn next(&mut self) -> Option<Self::Item> {
        // if let Some(ref mut remainder) = self.remainder {
        //     if let Some(next_delim) = remainder.find(self.delimeter) {
        //         let until_delimeter = &remainder[..next_delim];
        //         *remainder = &remainder[(next_delim + self.delimeter.len())..];
        //         Some(until_delimeter)
        //     } else {
        //         self.remainder.take()
        //     }
        // } else {
        //     None
        // }

        if let Some(remainder) = self.remainder {
            if let Some(next_delim) = remainder.find(self.delimeter) {
                let until_delimeter = &remainder[..next_delim];
                self.remainder = Some(&remainder[(next_delim + self.delimeter.len())..]);
                Some(until_delimeter)
            } else {
                self.remainder.take()
            }
        } else {
            None
        }
    }

The compiler doesn't give any error and work as expected. Is it less efficient as I'm moving out and then reassigning, or some rust idiomatic I'm missing?

@jonhoo
Copy link
Author

jonhoo commented Apr 6, 2024

@MaxAsif Either of those are fine. They both work here because the type inside the Option in self.remainder is Copy, and so therefore you can pull it out in the initial if let Some and still be allowed to overwrite it further down. That wouldn't be the case if the type wasn't Copy (e.g., if it were a String). In that case you'd have to use the former (i.e., the commented-out code).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment