Add getXxxAsByteArray() getters and overload with escape boolean#405
Conversation
|
Forgot to add: note that this has the unintended side-effect of exposing byte arrays outside of the record classes, which means that they could be changed outside of the class' control, hence making the class not fully immutable. I assume that this is OK as AFAIK this library does not make any hard contracts about class mutability, especially as this pattern of exposing the byte array already exists in the aforementioned An alternative is to return a read-only ByteBuffer wrapping our original byte array (see Another option is to copy the array each time: not great performance-wise, but data for a DNS record is quite limited anyway. |
ibauersachs
left a comment
There was a problem hiding this comment.
Thanks!
Besides the inline comments, please:
- Add a proper Javadoc to all new or changed methods, documenting the parameter
- Mark all new methods with
@since 3.6.5
As discussed in the issue below, the library currently exposes character strings only as escaped strings for textual representation. This is a limitation as users are forced to deal with unnecessary escapes for application logic, and are also limited to interpret bytes as UTF-16 (as `byteArrayToString` casts bytes to `char`), whilst RFCs do not state which encoding for bytes should be used. This commit adds: - `getXxxAsByteArray()` getters that expose the "raw" byte array, allowing full control to users. This follows the already established pattern in 'TXTBase` with the `getStringsAsByteArrays()` method - an overload of the existing getters, which allows to pass an `escape` boolean: `true` (the default) is the current behavior, while `false` simply converts the bytes to a String using the UTF-8 encoding Closes dnsjava#404
|
Sorry it took a while, I've pushed the changes requested in the comments. |
a601d27 to
378aa24
Compare
|
No worries. Thanks a lot! |
As discussed in the issue below, the library currently exposes character strings only as escaped strings for textual representation. This is a limitation as users are forced to deal with unnecessary escapes for application logic, and are also limited to interpret bytes as UTF-16 (as
byteArrayToStringcasts bytes tochar), whilst RFCs do not state which encoding for bytes should be used.This commit adds:
getXxxAsByteArray()getters that expose the "raw" byte array, allowing full control to users. This follows the already established pattern inTXTBasewith thegetStringsAsByteArrays()methodescapeboolean:true(the default) is the current behavior, whilefalsesimply converts the bytes to a String using the UTF-8 encodingCloses #404