0

I need to count duplicates of a custom struct in a Vec. I've found count partial duplicates in Vec of structs with a custom function.

My code is:

pub fn check_index_duplicates(
    dataset: &[main_index::MotiveImageDefinition],
) -> Result<(), Box<dyn std::error::Error>> {
    let mut keyed = HashMap::new();
    for c in dataset {
        keyed.entry(c.key()).or_insert(vec![]).push(c)
    }

    for (k, v) in &keyed {
        if v.len() > 1 {
            print_an_error(&(format!("Motive {:?} has {} duplicates on index.csv", k, v.len())));
        }
    }
    Ok(())
}

impl main_index::MotiveImageDefinition {
    fn key<'a>(&'a self) -> (&'a str, &'a str) {
        (&self.motive, &self.theme)
    }
}

#[derive(Debug)]
pub struct MotiveImageDefinition {
    pub id: u64,
    pub motive: String,
    pub theme: String,
    pub path: String,
    pub stereo_image: String,
    pub width_pix: String,
    pub height_pix: String,
}

It is exactly what I need. When I use clippy:

cargo clippy --all-targets --all-features -- -D warnings

It gives me the next two hints that I can't fix:

error: use of `or_insert` followed by a function call
   --> src/image/mod.rs:215:30
    |
215 |         keyed.entry(c.key()).or_insert(vec![]).push(c)
    |                              ^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(vec![])`

I tried to change or_insert to or_insert_with, but it doesn't compile.

Shepmaster
  • 388,571
  • 95
  • 1,107
  • 1,366
user1432966
  • 523
  • 1
  • 9
  • 20
  • 3
    Try `or_insert_with(Vec::new)` or `or_insert_with(|| vec![])`, whichever you find nicer. – user4815162342 Oct 05 '20 at 13:09
  • 2
    BTW I think clippy is being overzealous here - since an empty `Vec` doesn't allocate, creating an "unnnecessary" vec is transparent to the compiler. I strongly suspect the difference between the two variants would not be measurable. – user4815162342 Oct 05 '20 at 13:18
  • 1
    It's hard to answer your question because it doesn't include a [MRE]. We can't tell what crates (and their versions), types, traits, fields, etc. are present in the code. It would make it easier for us to help you if you try to reproduce your error on the [Rust Playground](https://play.rust-lang.org) if possible, otherwise in a brand new Cargo project, then [edit] your question to include the additional info. There are [Rust-specific MRE tips](//stackoverflow.com/tags/rust/info) you can use to reduce your original code for posting here. Thanks! – Shepmaster Oct 05 '20 at 13:35
  • It looks like your question might be answered by the answers of [Why should I prefer `Option::ok_or_else` instead of `Option::ok_or`?](https://stackoverflow.com/q/45547293/155423). If not, please **[edit]** your question to explain the differences. Otherwise, we can mark this question as already answered. – Shepmaster Oct 05 '20 at 13:36
  • @user4815162342 I'd suppose that (1) Clippy doesn't know (or maybe doesn't *want* to know) what `Vec::new` does (2) it's still the better practice to get into. However, it [looks like Clippy doesn't actually warn about OPs case](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=6a7ba473eb72009ebc39e802823f28c1) so perhaps it's already been fixed. – Shepmaster Oct 05 '20 at 13:36
  • @Shepmaster Thanks for looking into it. Although I understand the logic of (1), it's _also_ important for the lints to be reasonable, and `Vec::new` and `String::new` not allocating are an important part of Rust. So I support the decision not to warn in this case - great work from clippy devs! – user4815162342 Oct 05 '20 at 13:59
  • thanks @user4815162342 your answer works for me. – user1432966 Oct 05 '20 at 20:03

1 Answers1

1

What it actually work for me is @user4815162342 answer:

pub fn check_index_duplicates(
    dataset: &[main_index::MotiveImageDefinition],
) -> Result<(), Box<dyn std::error::Error>> {
    let mut keyed = HashMap::new();
    for c in dataset {
        keyed.entry(c.key()).or_insert_with(Vec::new).push(c)
    }

    for (k, v) in &keyed {
        if v.len() > 1 {
            print_an_error(&(format!("Motive {:?} has {} duplicates on index.csv", k, v.len())));
        }
    }
    Ok(())
}
user1432966
  • 523
  • 1
  • 9
  • 20