patterns icon indicating copy to clipboard operation
patterns copied to clipboard

consume as valid alternative to mem-replace idiom?

Open dekellum opened this issue 7 years ago • 3 comments

Before finding the mem-replace idiom here, I implemented something like the following working alternative for a_to_b, which also doesn't (surprisingly to me at first) require MyEnum to be Copy.

Requires re-assignment on the calling side but avoids needing mem::replace or cloning the name. At where I am in learning rust, I'm probably not yet qualified, but would it be helpful to include this as an alternative and listing pros/cons or limitations?

Thanks for compiling these patterns!

#[derive(Debug, PartialEq)]
pub enum MyEnum {
    A { name: String, x: u8 },
    B { name: String }
}

pub fn a_to_b(e: MyEnum) -> MyEnum {
    if let MyEnum::A { name, x: 0 } = e {
        MyEnum::B { name }
    } else {
        e
    }
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn it_works() {
        let mut e = MyEnum::A { name: "foo".to_owned(), x: 0 };

        e = a_to_b(e);
        assert_eq!(MyEnum::B { name: "foo".to_owned() }, e);

        e = a_to_b(e);
        assert_eq!(MyEnum::B { name: "foo".to_owned() }, e);
    }
}

dekellum avatar Feb 21 '18 22:02 dekellum

The reason to use mem::replace (or better mem::take where possible) is that it allows to do the change inside a structure when you only have a mutable reference. For example:

#[derive(Debug)]
enum MyEnum {
    A { name: String, x: i64 },
    B { name: String },
}

fn a_to_b(e: &mut MyEnum) {
    *e = if let MyEnum::A { ref mut name, x: 0 } = e {
        MyEnum::B {
            name: std::mem::take(name),
        }
    } else {
        return;
    }
}

#[derive(Debug)]
struct Outer(MyEnum);

fn main() {
    let name = "toto".to_string();
    let mut outer = Outer(MyEnum::A { name, x: 0 });
    let ref_outer = &mut outer;
    
    // this might be inside some function with ref_outer an argument
    a_to_b(&mut ref_outer.0);

    println!("{:?}", outer);
}

In this situation you just can't move the string like your function does. See also this Stackoverflow answer.

As a general remark, I think the examples should also include the use of the thing that is implemented, and entries need to discuss the purpose (apart from this one, I also has this issue with the entry on Default, being able to use mem::take() should IMHO be mentioned there).

starblue avatar Jan 04 '21 16:01 starblue

@starblue, your version can be further simplified as:

fn a_to_b(e: &mut MyEnum) {
    if let MyEnum::A { ref mut name, x: 0 } = e {
        *e = MyEnum::B { name: std::mem::take(name) }
    }
}

Looking back on this in 2021, it seems maybe there would be a way to provide a more unified series of examples starting from the trivial case of Copy types, to move case as I originally demonstrated, to &mut reference based, using mem::replace or take, as required. Note that there is now a clippy lint when mem::replace isn't strictly necessary.

The original example looks like it could use some modernization, at the very least.

dekellum avatar Jan 04 '21 17:01 dekellum

As a general remark, I think the examples should also include the use of the thing that is implemented, and entries need to discuss the purpose (apart from this one, I also has this issue with the entry on Default, being able to use mem::take() should IMHO be mentioned there).

mem::take is now mentioned. If you want to improve the page, PR are welcome!

dekellum, I like your simplification, you can submit a PR if you want :)

marcoieni avatar Jan 15 '21 23:01 marcoieni