KokaKiwi / rust-hex

A basic crate to encode values to hexadecimal representation. Originally extracted from rustc-serialize.
https://crates.io/crates/hex
Apache License 2.0
201 stars 55 forks source link

A couple ideas, both breaking changes #4

Closed frewsxcv closed 8 years ago

frewsxcv commented 8 years ago

Update FromHex::from_hex to accept &[u8] instead of &str.

It seems to be overkill to pass unicode into a function that operates on ASCII bits. The first operation that happens is converting the unicode to bytes anyways. Consider a couple scenarios:

Current design:

Vec::from_hex("hello")
Vec::from_hex(str::from_utf8(b"hello").unwrap())

With my proposal:

Vec::from_hex("hello".as_bytes())
Vec::from_hex(b"hello")

Stop accepting whitespace in the input to FromHex::from_hex

Is there any reason for this? I feel like if the user passes in superfluous characters that are not hex to from_hex, it should probably be result in an error.

Thoughts?

frewsxcv commented 8 years ago

I can make these changes if you think they're worth changing.

KokaKiwi commented 8 years ago

Update FromHex::from_hex to accept &[u8] instead of &str

Why not simply make FromHex::from_hex accept an argument of type S: AsRef<[u8]> which would allow both cases?

Stop accepting whitespace in the input to FromHex::from_hex

I don't really know, as accepting whitespace doesn't seem to have big drawbacks to me (regarding performances), but it doesn't feel right either now that you mentioned this.

In fact I don't really know the reasons for accepting whitespaces as it was already here where I extracted this code from the rustc-serialize crate (and seems to be still there)

PS: As it seems that "1234 5678".parse() doesn't accept whitespaces, maybe it would be alright to result in an error too indeed.

frewsxcv commented 8 years ago

Thanks!