NotificationsYou must be signed in to change notification settings
Fork515
Star3.8k

Avoid extra clone in config if possible#1137

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

sfackler merged 1 commit intorust-postgres:masterfromnyurik:use-string

Jul 6, 2024

Merged

Avoid extra clone in config if possible#1137

sfackler merged 1 commit intorust-postgres:masterfromnyurik:use-string

Jul 6, 2024

Conversation

Copy link

Contributor

nyurik commentedMay 2, 2024

Usingimpl Into<String> instead of&str in a fn arg allows both&str andString as parameters - thus if the caller already has a String object that it doesn't need, it can pass it in without extra cloning.

The same might be done with the password, but may require closer look.

Avoid extra clone in config if possible

3c6dbe9

Using `impl Into<String>` instead of `&str` in a fn arg allows both `&str` and `String` as parameters - thus if the caller already has a String object that it doesn't need, it can pass it in without extra cloning.The same might be done with the password, but may require closer look.

Copy link

Collaborator

sfackler commentedMay 2, 2024

I'm not super opposed to this, but at the same time I'm not sure it'll make a huge difference. What portion of runtime is spent constructing config?

Copy link

ContributorAuthor

nyurik commentedMay 2, 2024

@sfackler config specifically is not that big of a deal of course. My concern is that any frequently used code teaches users on how to do things - and in this case, IMO, the API should have only allowedString as the parameter, but because breaking API is not worth it, might as well introduce a backwards compatible solution.

Reasoning:

Function should require owned value when it needs to keep it - to let caller know how to use it and optimize for it
API recommendations for builders - suggesting the structure of APIs
My question to community on how to do this types of API -- lots of good responses there

P.S. community recommended using explicit generic rather thanimpl Into<String> as a param type.