Avoid double `clone()`

help
#1

How do I avoid having to clone() twice? I am using clone() to turn &String into String. Alternative solutions are welcome.

fn parse(mut lexer: Lexer) -> Vec<Expr> {
    let mut expr_buffer = Vec::new();
    let mut label = String::new();

    while let Some(token) = lexer.next() {
        match token.as_str() {
            "$}" => break,
            "$c" => {
                let constants = parse_until(&mut lexer, "$.");

                let constants = constants.iter()
                    .map(|constant| Expr::Constant(constant.clone())); // First `clone()`.

                expr_buffer.extend(constants)
            }
            token => label = token.to_owned(),
        }
    }

    return expr_buffer
}

fn parse_until(lexer: &mut Lexer, end: &str) -> Vec<String> {
    let mut string_buffer = Vec::new();

    while let Some(token) = &lexer.next() {
        if token == end {
            return string_buffer
        }

        string_buffer.push(token.clone()) // Second `clone()`.
    }

    panic!()
}
#2

I am not sure if the following is any better, but it doesn't have multiple clone()s.

fn parse(mut lexer: Lexer) -> Vec<Statement> {
    let mut expr_buffer = Vec::new();
    let mut label = String::new();

    while let Some(token) = lexer.next() {
        match token.as_str() {
            "$}" => break,
            "$c" => expr_buffer.extend(parse_until(&mut lexer, "$.", |c| Statement::Constant(c.clone()))),
            token => label = token.to_owned(),
        }
    }

    return expr_buffer
}

fn parse_until<T, F>(lexer: &mut Lexer, end: &str, f: F) -> Vec<T>
where
    F: Fn(&String) -> T
{
    let mut string_buffer = Vec::new();

    while let Some(token) = &lexer.next() {
        if token == end {
            return string_buffer
        }

        string_buffer.push(f(token))
    }

    panic!()
}

Am I understanding something wrong here? Was there a reason to avoid two clone()s in the first place?

1 Like
#3

What's lexer.next() returning? If it's a String, then the answer is to just stop turning it into a &String and you can stop cloning it. (And it it's returning a &str of some sort, then you don't need the & in &lexer.next().)

2 Likes
#4

Assuming the lexer returns owned strings (i.e. lexer.next() -> Option<String>), if you just drop the & in &lexer.next(), token will have type String in the loop in parse_until, and then you can just push it into the vector and return a Vec<String>, which solves the first clone. For the second one, you should use constants.into_iter() instead of constants.iter(), and then it will consume the strings instead of simply getting a reference, so you will be able to just do Expr::Constant(constant) instead of cloning.

There is also a way to solve this where you use references everywhere, making the types Expr<'a> and Lexer<'a> borrowing from some backing store, and then you don't have to do any copies. But this is trickier and more verbose (and may not be applicable depending on where the lexer is getting its data from).

2 Likes
#5

Thank you very much for the reply!

Indeed. The return type of lexer.next() is Option<String> (in fact, Lexer is an Iterator).

#6

Thank you very much! Lexer is opening files and reading them, so I guess it makes sense for it to own the Strings, as the caller only passes a Path when constructing it.

#7

Yes, in that case I think it is best for the Expr to own the strings in the end, so you don't need the <'a> stuff.