On: Effective Rust (2021)

[boredumb]

Thanks! I've been writing quite a lot of rust lately but have been mostly silo'd to my own so i'll definitely read through and see how wildy un-idiomatic my code is.

Even just scanning through it I saw `Avoid matching Option and Result` and the `if let` stuff that I have NOT been using and it looks much more concise already.

[bryanlarsen → boredumb]

Pedantic clippy will suggest those changes, amongst others. I wouldn't recommend running pedantic clippy in CI, but it is useful to run it against your app occasionally.

[boredumb → bryanlarsen]

Very cool, running it on the project I have open immediately started barfing at me over using 'return' in quite a few spots.

[harpiaharpyja → boredumb]

Occasionally matching Option or Result is actually the most readable way to express some logic. But it's definitely not the first thing you should reach for.

When I first started using Rust I did make the mistake of matching Option and Result everywhere and when I learned better I did cut out quite a bit of verbosity and unnecessary nesting by going back and refactoring those in code I had written. But that doesn't mean you should never use match with them. I thought I should share this experience for anyone new to the language reading this.

[weavie → boredumb]

The example at the end of that section:

        pub fn encrypted(&self) -> Vec<u8> {
            encrypt(self.payload.as_ref().unwrap_or(&vec![]))
        }

Needlessly allocates a `Vec` if `self.payload` is `Some`. Really you should do:

     encrypt(&self.payload.as_ref().unwrap_or_else(|| &vec![]))

But that isn't going to work because the `else` Vec will be dropped at the end of the closure.

In this situation (if the code was on a hot path where every bit of performance mattered) I would probably do something like:

    match self.payload.as_ref() {
        Some(payload) => encrypt(payload),
        None => encrypt(&vec![])
    }

There is a slight bit of duplicated code. I'd be curious if anyone could come up with a better way.

[tveita → weavie]

From https://doc.rust-lang.org/std/vec/struct.Vec.html

  if you construct a Vec with capacity 0 via Vec::new, vec![], Vec::with_capacity(0), or by calling shrink_to_fit on an empty Vec, it will not allocate memory.

So an empty vec![] is just a struct on the stack; very cheap to make, and easy for the compiler to optimize out if it can see that it's not used in some paths.

[jpochyla → weavie]

This is... not how I would really write it. If `encrypt` takes a `&[u8]`, as it should, you don't need a `&vec![]` at all, and can do:

  self.payload.as_deref().unwrap_or_default()

...which I think is the best here.

[umanwizard → weavie]

> Needlessly allocates a `Vec` if `self.payload` is `Some`.

Empty vecs don’t require a heap allocation, so your original code is actually fine. In release mode it should compile to exactly the same instructions as your last example.

[masklinn → weavie]

> Needlessly allocates a `Vec` if `self.payload` is `Some`.

Vec is documented not to allocate if empty (unless created with a set capacity). So all the code does is store 3 u64 on the stack.

[laeri → weavie]

Can you also use `unwrap_or_default` in this example?

[Omin → laeri]

No, because it requires an &Vec<_> and that doesn't implement Default and for good reason. Just ask yourself, where would the default empty vec live so that you could create a reference to it.

When using unwrap_or(&vec![]), it lives in the enclosing stack. Without the reference, you could use unwrap_or_default().

[laeri → Omin]

Thanks for your reply that cleared it up. Was a bit confused why the `unwrap_or(&vec![])` worked but as you said it lives in the enclosing stack.

[HankB99 → weavie]

I lack Rust experience to comment on your suggestion but it does make me wonder if the author has provided a means for feedback in order to make this "More Effective Rust" (No pun intended if you're familiar with Meyers' followup book.)

[weavie → HankB99]

The other commenters to this are correct, `Vec::new()` doesn't actually allocate so the code in the book is fine. I had a brain fart.

If that function was taking a reference to an object that was fairly expensive to create a default value for then my conundrum would still exist.

[keyle → boredumb]

That is something I did naturally coming from Swift.

There is a growing bunch of people I've met that said the match stuff is overused.