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

Measurement constructors should take owned data #16

Open
rillian opened this issue Mar 15, 2022 · 0 comments
Open

Measurement constructors should take owned data #16

rillian opened this issue Mar 15, 2022 · 0 comments

Comments

@rillian
Copy link
Contributor

rillian commented Mar 15, 2022

Currently Measurement::new() takes an &[u8] argument. Since this type is just a wrapper around Vec<u8> it would be better to take an owned Vec directly.

Because the type needs an owned copy of the data, when passed a reference it must immediately clone it. This is fine if the caller also needs a copy. If it's constructed just to pass in and is not needed otherwise, the clone is unnecessary extra work. This can be avoided by taking the vector by value. If the caller needs to retain a copy, it can explicitly clone.

let data: &[u8] = get_intput();
let m = Measurement::new(data.clone());

This makes it clear in the API contract that an allocation is happening, and avoiding it is up to the caller.

The impl From<&str> would then make its own clone call, as an immediate example.

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

No branches or pull requests

1 participant