Multiaddr refactor #217
Multiaddr refactor #217
Conversation
| private val NO_PARSER: (Protocol, String) -> ByteArray = { protocol, _ -> | ||
| throw IllegalArgumentException("No value serializer for protocol $protocol") | ||
| } | ||
| private val NO_DESERIALIZER: (Protocol, ByteArray) -> String = { protocol, _ -> |
There was a problem hiding this comment.
I'm confused about the PARSER vs DESERIALIZER here. In my mind a PARSER goes from String to internal representation and a SERIALIZER goes from internal to String. A PARSER and DESERIALIZER would then be the same thing.
There was a problem hiding this comment.
Oh, that was also breaking my mind.
I used to term 'serializer/deserializer' for converting 'bytes' from/to internal representation
'parser' in my mind is 'string => internal representation' converter.
If we assume that 'bytes' is internal form, then it should be something opposite to 'parser'. 'Stringifier', 'Composer', 'Unparser'? What term do you think is the most adequate?
Just don't want to confuse it with the 'binary serializer'. If take the Multiaddr class for example then it clearly has 3 representations: binary, string and internal. And they are all distinct
There was a problem hiding this comment.
In other words: what should be the name of a substance who does toString()? 😄
There was a problem hiding this comment.
oof 3 meaningful states makes it hard. While it's not valid English, Stringifier is probably a good name as it's very clear what it does. Or possibly parser becomes STRING_TO_BYTES and the current deserialiser becomes BYTES_TO_STRING?
There was a problem hiding this comment.
I've stick with stringifier: I found a number of existing classes with this name. Even IDEA vocabulary knows this term! 🎉
STRING_TO_BYTES sounds more rigorously but it would ideally be STRING_TO_BYTES_CONVERTER which is already too long
| addr.toByteArray(StandardCharsets.UTF_8) | ||
| } | ||
| private val UTF8_DESERIALIZER: (Protocol, ByteArray) -> String = { _, bytes -> | ||
| String(bytes, StandardCharsets.UTF_8) |
There was a problem hiding this comment.
I don't think this will reject invalid characters (just replace them with ?). If we want to fully validate we could use StandardCharsets.UTF_8.newDecoder().onMalformedInput(CodingErrorAction.REPORT).onUnmappableCharacter(CodingErrorAction.REPORT).decode() but that adds a bunch of complexity and I'm not entirely sure it's worth it.
There was a problem hiding this comment.
Right, better be safe. However I've added a UTF-8 validator instead: 6375f2a
| private val NO_PARSER: (Protocol, String) -> ByteArray = { protocol, _ -> | ||
| throw IllegalArgumentException("No value serializer for protocol $protocol") | ||
| } | ||
| private val NO_DESERIALIZER: (Protocol, ByteArray) -> String = { protocol, _ -> |
There was a problem hiding this comment.
oof 3 meaningful states makes it hard. While it's not valid English, Stringifier is probably a good name as it's very clear what it does. Or possibly parser becomes STRING_TO_BYTES and the current deserialiser becomes BYTES_TO_STRING?
Multiaddrto be represented by a list ofMultiaddrComponentinstead ofPairProtocolenum implementation. Add component value validationMultiaddrmethods/constructorsNoiseXXSecureChannelupdate to make it more extendableNote: breaking API changes