Conversation
| impl HexWriter { | ||
| /// Creates a new [`HexWriter`] specify the `capacity` of the inner `String` that will contain | ||
| /// the hex value | ||
| pub fn new(capacity: usize) -> Self { |
There was a problem hiding this comment.
I think new should take no parameters and we should have a separate with_capacity constructor which takes a capacity.
There was a problem hiding this comment.
I think, since the string must be created with the capacity then we should only have a single constructor. Seems to me that new is correct here.
/// Note that to achieve better perfomance than [`ToHex`] the struct must be
/// created with the right `capacity` of the end hex string result so that the
/// inner `String` doesn't re-allocate
There was a problem hiding this comment.
What mentioned @tcharding is the reason why I avoided having the new without parameters. @apoelstra do you think it's okay?
|
concept ACK, though CI is failing |
|
CI fail is because you include |
this structs create an hex string from a writer, in cases where you have an encodable object it allows skipping the creation of the intermediate Vec<u8>
|
@tcharding are your nits addressed? |
|
Yep, thanks for checking with me! |
3 participants
This struct create an hex string from a writer, in cases where you have
an encodable object this allows skipping the creation of the intermediate
Vec<u8>Useful in rust_bitcoin
serialize_hexhttps://github.com/rust-bitcoin/rust-bitcoin/blob/0e82376bf85137494ff220608e80b9c78c1932b5/src/consensus/encode.rs#L149 but needs rust-bitcoin/rust-bitcoin#1027 so that result string size is known ahead of time