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.

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

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.

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.

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.

Can you also use `unwrap_or_default` in this example?

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().

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.

> 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.

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.)

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.

> 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.

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.

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.