5 ms·

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 cod

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 3y ago
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 3y ago
  Very cool, running it on the project I have open immediately started barfing at me over using 'return' in quite a few spots.
weavie 3y ago
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 3y ago
  From https://doc.rust-lang.org/std/vec/struct.Vec.html 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.
- deleted 3y ago
  [deleted]
- laeri 3y ago
  Can you also use `unwrap_or_default` in this example?
  - Omin 3y ago
    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 3y ago
      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.
- jpochyla 3y ago
  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.
- School-Cotton 3y ago
  > 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.
- HankB99 3y ago
  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 3y ago
    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.
- masklinn 3y ago
  > 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.
keyle 3y ago
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.
harpiaharpyja 3y ago
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.

bryanlarsen 3y ago

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 3y ago
Very cool, running it on the project I have open immediately started barfing at me over using 'return' in quite a few spots.

weavie 3y ago

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 3y ago
From https://doc.rust-lang.org/std/vec/struct.Vec.html 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.
deleted 3y ago
[deleted]
laeri 3y ago
Can you also use `unwrap_or_default` in this example?
- Omin 3y ago
  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 3y ago
    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.
jpochyla 3y ago
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.
School-Cotton 3y ago
> 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.
HankB99 3y ago
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 3y ago
  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.
masklinn 3y ago
> 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.

keyle 3y ago

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.

harpiaharpyja 3y ago

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.