Add impl Encode for [T], where T: Encode #542
Conversation
|
I think this will conflict with #526 and I want to get that PR merged in first, so this PR will have to stay open for a while. Thank you for your contribution regardless! |
|
#526 is now merged. Looks like this PR is still valid. You can remove the implementation on |
Since Encode takes a reference, this allows us to encode &[T] directly using this implementation. The encoding scheme is the same as for Vec<T>. This also makes the implementation for &[u8] superfluous, since we get an implementation for [u8] by virtue of u8 implementing encode. This also gives us free implementations for &[u16], &[u32], etc. which is quite useful. Nonetheless, we keep the implementation for &[u8] around, because the implementation can directly write a large number of bytes, which can be more efficient than the generic implementation.
Since we've implemented Encode for [T], this makes the implementation for Box<[T]> redundant (since we have a blanket implementation for Box<T>), and ditto for &[T], which this change replaces by combining the implementations for [T] and &T.
|
Nice! I've removed redundant implementations for |
Codecov Report
@@ Coverage Diff @@
## trunk #542 +/- ##
==========================================
- Coverage 54.52% 54.47% -0.06%
==========================================
Files 49 49
Lines 4422 4417 -5
==========================================
- Hits 2411 2406 -5
Misses 2011 2011
Continue to review full report at Codecov.
|
|
Thanks! |
Since Encode takes a reference, this allows us to encode &[T] directly
using this implementation. The encoding scheme is the same as for
Vec.
This also makes the implementation for &[u8] superfluous, since we get
an implementation for [u8] by virtue of u8 implementing encode. This
also gives us free implementations for &[u16], &[u32], etc. which is
quite useful. Nonetheless, we keep the implementation for &[u8] around,
because the implementation can directly write a large number of bytes,
which can be more efficient than the generic implementation.
And uh, apologies if you would have preferred I raise an issue first, but this felt like a small enough change to warrant a direct PR.
I actually ran into a situation where this was convenient. My workaround to not having this was to take
&Vec<T>as an argument to my function, and this seemed highly unidiomatic. I was also surprised not to see this when digging around in the docs the other day.