Help with struct containing String fields

[svilen]

Hello everyone,
Another newbie question from me.

Consider this snippet of code:

use chrono::{DateTime, Utc};

#[derive(PartialEq, Debug)]
struct DealEvent {
  pub r#type: String,
  pub value: String,
  pub inserted_at: DateTime<Utc>,
}

fn filter_by_type(deal_events: Vec<DealEvent>) -> Vec<DealEvent> {
  let result = deal_events
    .into_iter()
    .filter(|e| e.r#type == String::from("new"))
    .collect();

  result
}

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

  #[test]
  fn filter_by_type_filters_by_type() {
    let bad_de = DealEvent {
      r#type: String::from("foo"),
      value: String::from("bar"),
      inserted_at: Utc::now(),
    };

    let good_de = DealEvent {
      r#type: String::from("new"),
      value: String::from("bar"),
      inserted_at: Utc::now(),
    };

    let result = filter_by_type(vec![bad_de]);
    assert_eq!(result, vec![]);

    let result = filter_by_type(vec![good_de]);
    assert_eq!(result, vec![good_de]);
  }
}

This snippet produces the following error:

error[E0382]: use of moved value: `good_de`
  --> src/lib.rs:45:29
   |
35 |     let good_de = DealEvent {
   |         ------- move occurs because `good_de` has type `DealEvent`, which does not implement the `Copy` trait
...
44 |     let result = filter_by_type(vec![good_de]);
   |                                      ------- value moved here
45 |     assert_eq!(result, vec![good_de]);
   |                             ^^^^^^^ value used here after move

I conceptually understand what the error is about, but I'm not sure how to fix it. I'd tried adding the Copy and Clone traits to the struct, but it seems that Copy is not implemented for String types, so that didn't work.

I would really appreciate if someone can help me not only fix this, but explain why this is happening, so I can better understand how the compiler thinks.

Thank you!

P.S. As a secondary question — is there a way not to do let result = ... and then return result in the filter_by_type function? I tried using the chain of functions directly, but then the compiler complained that the expected return type was () I figured this out just now — I put semicolon in the end after collect() which made the function return () rather than the Vec

[FenrirWolf]

Strings aren't Copy, which is why you can't derive that. They are Clone though. But it isn't enough to just derive Clone, you also have to manually invoke the associated clone() method like so:

Rust Playground

[svilen]

@FenrirWolf Amazing, thank you so much! It makes sense. I noticed one small thing you've changed, the comparison in filter:

.filter(|e| e.r#type.as_str() == "new")

Is this because this is faster than allocating a String?

[mbrubeck]

Yes, though the as_str isn't needed. You can write:

.filter(|e| e.r#type == "new")

Playground.

[svilen]

@mbrubeck Nice!! That's even neater.

Another, last question: is using .clone() something people do often, something to avoid or it just depends and I shouldn't worry about it? I know this is just used in the unit test code, but I'm wondering whether there is something I should know if I decide to use it in a different context? Thanks!

[OptimisticPeach]

.clone on an object can vary in effect. usize::clone is very different to Vec::<T: Clone>::clone. In general though, Copy is something that can be bitwise copied, that is, it is safe to just do a memcpy on it. .clone has user code in it, which can do more complicated things, such as copy the contents of a Vec into another, new Vec, each by each. Since String is actually backed by a Vec<u8>, String::clone would be almost identical to Vec::<u8>::clone.
So in actuality it depends. If you're in a one-off time use of it, then the optimization's probably not worth it, but if you're using them more liberally then perhaps a reconsideration of it would make sense. Also, when in a generic context, IE T: Clone, then you should regard T as possibly having anything that could be cloned in some way, from a usize to a Vec<Vec<Vec<u8>>>.