Key deserializer improvements #467
Conversation
Add as_slice as default trait impl
|
Now it's better. |
packages/storage-plus/src/de.rs
Outdated
| @@ -191,18 +139,13 @@ impl<T: KeyDeserialize, U: KeyDeserialize> KeyDeserialize for (T, U) { | |||
|
|
|||
| Ok((T::from_slice(t)?, U::from_slice(u)?)) | |||
There was a problem hiding this comment.
hmmm.. could you use from_vec here as well?
I guess you would need value.split_off rather than split_at. But should be more consistent/efficient
packages/storage-plus/src/de.rs
Outdated
| @@ -221,11 +164,6 @@ impl<T: KeyDeserialize, U: KeyDeserialize, V: KeyDeserialize> KeyDeserialize for | |||
|
|
|||
| Ok((T::from_slice(t)?, U::from_slice(u)?, V::from_slice(v)?)) | |||
There was a problem hiding this comment.
Same as the (T, U) tuples (from_vec)
There was a problem hiding this comment.
Did it. It's a little unclear with the var names. And, I doubt it's more efficient.
If we wanted a self-contained version (that is, one that doesn't depend on from_slice), we could always use from_vec(t.to_vec()) and still work with slices / references in the mean time.
There was a problem hiding this comment.
In fact, for triples, the version with split_off() has four vec allocations instead of three; because of the small vec for the length of v. Plus, it requieres a conversion to to_slice() for calling try_into(). Plus mut vecs.
I really like the previous version better.
|
So, I guess you prefer the version with |
Why is it better? Easier to read code? |
|
OK then. |
Closes first part of #464.