aherbert commented on issue #46: URL: https://github.com/apache/commons-codec/pull/46#issuecomment-617212512
I think this is not the way to go. I would not use the Hex class at all. Use a decode table as is done for Base32/64Codec. I would drop support for the Charset. This is only relevant when converting to/from String via char[] arrays. You should not even have to deal with char[] let alone allocate them as full length intermediates for every encode or decode of a byte[]. Plus there are extra public static methods added to Base16Codec which duplicate functionality from Hex. Hex has 16 characters. I would expect to construct a Base16Codec with a boolean flag for upper or lowercase. In Base32 we support decode of Base32 or Base32 Hex alphabets. Base64 supports URL safe or standard MIME 64 alphabet. So I see Base16 supporting encoding to lower or upper case and decoding either, effectively making decoding case insensitive. The BaseNCodec is purely about packing bytes using an alphabet of N characters expressed using UTF-8 characters, but specifically UTF-8 characters less than 127 so effectively the standard ASCII alphabet. This is enforced by the base class BaseNCodec which converts Strings to/from bytes as UTF-8. Any extension of BaseNCodec should only deal with encoding/decoding between packed bytes and a byte alphabet (that maps in UTF-8 to a readable character set). I would expect the class to look like Base32Codec. Essentially copy that class and just remove some of the extra alphabet support. This would seamlessly support decoding of lower and upper case hex characters using a decode table. Any character not in the decode table is ignored as whitepsace/padding. Your current class cannot decode whitespace padded Hex or encode with a line wrapping separator. I would expect to be able to encode to Hex wrapped at 72 characters: ``` ABCDEF0123456789..... ABCDEF0123456789..... ABCDE ``` and then decode this. Currently the functionality in the Base16 class has a different extended API than Base32/64 (more public static methods) but less flexible encoding or decoding with regard to whitespace. Ideally we want to be able to use Base16 to produce ASCII armoured hex encoding of byte data as can be done using the Base32/64 variants. This is not an extension of BaseNCodec to support Base16. It is a shoehorn of ported functionality into the same namespace. Anyone viewing the 3 classes down the line will wonder why the functionality and API are not aligned. Let's reduce maintenance burden going forward by making Base16 look and function like Base32/64. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
