garydgregory commented on issue #46: URL: https://github.com/apache/commons-codec/pull/46#issuecomment-617215257
Good info here Alex, thank you. May you please update the BaseNCodec Javadoc if it is missing some of this good info? Gary On Tue, Apr 21, 2020 at 10:23 AM Alex Herbert <notificati...@github.com> wrote: > 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. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <https://github.com/apache/commons-codec/pull/46#issuecomment-617212512>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AAJB6N2PZX6BJ4RJFVKT6HDRNWT6LANCNFSM4MLZXUVA> > . > ---------------------------------------------------------------- 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
