Hands-on Rust: clone_dirty cloning HashSet

Title: Hands-On Rust (Chapter 10)

This has been bugging me a bit since coding it, so I tried the change and it seems to work.

Isn’t it a bit expensive to clone a HashSet that you intend to immediately replace?

I tried the following change to clone_dirty(&self) -> Self

    pub fn clone_dirty(&self) -> Self {
        // let mut cloned = self.clone(); // TODO: isn't this expensive??
        // cloned.is_dirty = true;
        // cloned
        Self {
            visible_tiles: HashSet::new(),
            radius: self.radius,
            is_dirty: true,
        }
    }

Game play seems to be working fine so far. I’m assuming that creating a new HashSet is less expensive than closing a populated HashSet. That also raised the question for me as to whether visible_tiles should be Option instead and the None would serve the same purpose as is_dirty. Maybe I’ll see that this is a bad idea as I progress…

Marked As Solved

herbert

Author of Hands-on Rust

That’s a good point; clone can be expensive (it’s remarkable how well LLVM optimizes it out, especially on release builds). There’s a bit of compiler magic that notices that it’s basically a set of memcpy calls under the hood, and elides them. I like your approach better, though - it’s dangerous to rely on the compiler noticing an optimization when you can tell it what you want.

Adding an is_visible method is also a great idea. I’ll see if I can refactor the book code a little.

I’ll try and merge this into the next beta. Thank you!

Post #3

Where Next?