Skip to content
This repository has been archived by the owner on Nov 25, 2018. It is now read-only.

Use &[u8] instead of &Vec<u8> #6

Merged
merged 1 commit into from
Jul 1, 2016
Merged

Conversation

panicbit
Copy link
Contributor

No description provided.

@tarcieri
Copy link
Member

tarcieri commented Jul 1, 2016

Thanks for the PR. This covers some but not all of the cases. Also the format is about to change. But since master is in a state of flux anyway, I'll call this an improvement.

@tarcieri tarcieri merged commit 8a8cd4d into cryptosphere:master Jul 1, 2016
@panicbit
Copy link
Contributor Author

panicbit commented Jul 1, 2016

Not sure what you mean exactly. Did I miss some &Vec somewhere?

@panicbit panicbit deleted the patch-1 branch July 1, 2016 01:37
@tarcieri
Copy link
Member

tarcieri commented Jul 1, 2016

Yes, there are several fields still using Vec<u8>, like identifier and location

@panicbit
Copy link
Contributor Author

panicbit commented Jul 1, 2016

No, those are fine. You'll want to consume the identifier and probably also the location.

@tarcieri
Copy link
Member

tarcieri commented Jul 1, 2016

So playing the devil's advocate: why can't they be immutably borrowed as a &[u8]? (and use e.g. std::borrow::Cow)

@panicbit
Copy link
Contributor Author

panicbit commented Jul 1, 2016

&[u8] would be bad because it would either force you to
a) clone the data, even if the caller doesn't need the original data anymore (unnecessary clone) or
b) add a lifetime parameter to Token (reduced usability)

Cow would allow you to pass either an owned or a borrowed value, but using ToOwned or Into instead would probably be much more ergonomic (the caller wouldn't need to explicitly clone anymore or explicitly create a Cow).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants