rust-unofficial / patterns

A catalogue of Rust design patterns, anti-patterns and idioms
https://rust-unofficial.github.io/patterns/
Mozilla Public License 2.0
7.98k stars 363 forks source link

Adding 'clone to satisfy the borrow checker' anti-pattern #110

Closed simonsan closed 3 years ago

simonsan commented 3 years ago

Due to inactivity of the author and maintainers not being able to edit the PR I'll move #23 to a new branch in this repository.

Future fixes to the PR will be made on this branch.

pickfire commented 3 years ago

That said, I have seen quite a few blog posts discussing "it is okay to write inefficient code". Those posts even mentioned that clone can be used this way. I wonder if it should be put into the anti-pattern section, we could tell that we should reduce the usage of clone, but I think putting it in anti-pattern is going to make things hard (like what it did to way-cooler).

simonsan commented 3 years ago

That said, I have seen quite a few blog posts discussing "it is okay to write inefficient code". Those posts even mentioned that clone can be used this way. I wonder if it should be put into the anti-pattern section, we could tell that we should reduce the usage of clone, but I think putting it in anti-pattern is going to make things hard (like what it did to way-cooler).

I think it's fine to consider it an anti-pattern, as it's not trying to state that clone is inherently bad, but it is an anti-pattern in regards to satisfy the borrow checker. As long as it is explicitly stated and clear to the reader that this is the topic, it should be ok, I think.

simonsan commented 3 years ago

What I personally still miss here is the influence on performance of clone when used deliberately with bigger Vecs for example. I think that could be stated as a Disadvantage or outcome when using this anti-pattern often. While it should also explicitly state that clone is fine and what I mentioned before, the scope of this anti-pattern should be made crystal clear.

LukeMathWalker commented 3 years ago

My two cents - along the lines of what @pickfire mentioned: we should be very wary of demonizing .clone usage, particularly when we are talking about beginners.
At the same time, it makes sense to call out that "cloning your way out" might work as a short-term solution but might hide bigger issues with the whole architecture of your code that you might have to confront down the line anyway. Maybe linking a balanced article on the topic?

simonsan commented 3 years ago

Maybe linking a balanced article on the topic?

Do you have any recommendations?

LukeMathWalker commented 3 years ago

http://xion.io/post/code/rust-borrowchk-tricks.html perhaps?

Djazouli commented 3 years ago

Hello ! Just to let you know, there is a note at the bottom of the file idioms/mem-replace.md, that referenced to PR #23 , and I believe the changes related to this note should then be included in this PR (I believe the only thing needed is to add a link to the newly created file)

simonsan commented 3 years ago

@pickfire @MarcoIeni Worked in your proposals, want to read over it again? ;)

MarcoIeni commented 3 years ago

That's great, thanks!