Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change unnecessary usage of Arc to borrow instead. #1071

Merged
merged 1 commit into from
Mar 4, 2023

Conversation

ThomasFrans
Copy link
Contributor

Some basic cleanup of function signatures that took ownership of their parameters, even though they didn't need ownership. Switching over the usage of Arc to a normal borrow has the added benefit of cleaning up the code a bit since now a reference can be given instead of having to clone the values. The other benefit is that a lot of clones aren't necessary anymore. It's not going to have noticable performance benefits, but it is still a good thing to have less clones all over the code.

Some basic cleanup of function signatures that took ownership of their
parameters, even though they didn't need ownership. Switching over the
usage of `Arc` to a normal borrow has the added benefit of cleaning up
the code a bit since now a reference can be given instead of having to
clone the values. The other benefit is that a lot of clones aren't
necessary anymore. It's not going to have noticable performance
benefits, but it is still a good thing to have less clones all over the
code.
@ThomasFrans
Copy link
Contributor Author

I hope this sort of general cleanup is accepted. I try not to do it for small stuff as it's probably annoying to have 100 small PRs with general cleanup. I only did this one as it also makes the code more understandable in my opinion as now it is clear that all those functions don't take ownership of their values. If this isn't ok, let me know 🙂

@hrkfdn
Copy link
Owner

hrkfdn commented Mar 4, 2023

Very nice, thank you! Just a few days ago I wondering whether this could be improved.

@hrkfdn hrkfdn merged commit 98a0596 into hrkfdn:main Mar 4, 2023
@ThomasFrans ThomasFrans deleted the remove-unnecessary-arc branch March 7, 2023 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants