Commons Codec
  1. Commons Codec
  2. CODEC-21

[codec] Alterations to Binary.java and its unit test for 1.3 release

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2
    • Fix Version/s: 1.3
    • Labels:
      None
    • Environment:

      Operating System: other
      Platform: All

      Description

      • BIT_n constants made private
      • Used BITS in for loop to make code shorter and cleaner
      • Added two new methods for encoding and decoding to BitSet
        curtesy of Vish Krishnan - sorry Vish I had to change some
        things on your contribs.
      • Made 100% test coverage as validated by clover
      1. ASF.LICENSE.NOT.GRANTED--patch.jar
        7 kB
        Alex Karasulu
      2. ASF.LICENSE.NOT.GRANTED--BinaryTest-BitSet.patch
        17 kB
        ggregory@seagullsw.com
      3. ASF.LICENSE.NOT.GRANTED--Binary-BitSet.patch
        4 kB
        ggregory@seagullsw.com

        Activity

        Hide
        Alex Karasulu added a comment -

        Created an attachment (id=10882)
        jar compressed file containing BinaryAndTest.patch

        Show
        Alex Karasulu added a comment - Created an attachment (id=10882) jar compressed file containing BinaryAndTest.patch
        Hide
        ggregory@seagullsw.com added a comment -

        Patch partially applied and modified:

        (1) The bitset methods and tests do not compile on <1.4. I've fixed that in
        another patch attached to this ticket if/when we want to use it here.

        (2) I did not include the bit set methods in this pass as they seem to be out of
        context for this class but would like to discuss this. The Binary codec as is
        today converts between byte arrays and "0/1" strings. If we want to convert to
        and from BitSet objects, maybe this is another codec or a BitSetUtils class in
        [lang]?

        (3) All Java strings are Unicode, so the use of "ASCII" in comments and method
        names is misleading IMO, especially when what is meant are "Strings of 0s and
        1s" so I'd like to not use "Ascii" in method names. I'll try to address that on
        commons-dev.

        (4) I've also renamed the instance method "byte[] decode(Object)" to
        "toByteArray" because the name "decode" was being overloaded but not overriden,
        which to makes the class more confusing to understand IMHO considering the codec
        interfaces Decoder, StringDecoder, and BinaryDecoder. I found this tricky since
        Binary is NOT a StringDecoder but a BinaryDecoder. Using "decode" the method
        look like a framework implementation method when it is not. This is all I
        suppose a reflection on how truly useful these interfaces are in the first
        place, a different topic I know.

        Show
        ggregory@seagullsw.com added a comment - Patch partially applied and modified: (1) The bitset methods and tests do not compile on <1.4. I've fixed that in another patch attached to this ticket if/when we want to use it here. (2) I did not include the bit set methods in this pass as they seem to be out of context for this class but would like to discuss this. The Binary codec as is today converts between byte arrays and "0/1" strings. If we want to convert to and from BitSet objects, maybe this is another codec or a BitSetUtils class in [lang] ? (3) All Java strings are Unicode, so the use of "ASCII" in comments and method names is misleading IMO, especially when what is meant are "Strings of 0s and 1s" so I'd like to not use "Ascii" in method names. I'll try to address that on commons-dev. (4) I've also renamed the instance method "byte[] decode(Object)" to "toByteArray" because the name "decode" was being overloaded but not overriden, which to makes the class more confusing to understand IMHO considering the codec interfaces Decoder, StringDecoder, and BinaryDecoder. I found this tricky since Binary is NOT a StringDecoder but a BinaryDecoder. Using "decode" the method look like a framework implementation method when it is not. This is all I suppose a reflection on how truly useful these interfaces are in the first place, a different topic I know.
        Hide
        ggregory@seagullsw.com added a comment -

        Created an attachment (id=10885)
        Patch to Binary v.1.8 for bit set methods.

        Show
        ggregory@seagullsw.com added a comment - Created an attachment (id=10885) Patch to Binary v.1.8 for bit set methods.
        Hide
        ggregory@seagullsw.com added a comment -

        Created an attachment (id=10886)
        Patch to BinaryTest v.1.9 for BitSet methods.

        Show
        ggregory@seagullsw.com added a comment - Created an attachment (id=10886) Patch to BinaryTest v.1.9 for BitSet methods.
        Hide
        Michael Heuer added a comment -

        Re: 2)

        COM-1068 contains a BitSetUtils class submitted to [lang].

        Show
        Michael Heuer added a comment - Re: 2) COM-1068 contains a BitSetUtils class submitted to [lang] .
        Hide
        ggregory@seagullsw.com added a comment -

        Set to FIXED. The class BinaryCodec seems ready for release.

        Show
        ggregory@seagullsw.com added a comment - Set to FIXED. The class BinaryCodec seems ready for release.

          People

          • Assignee:
            Unassigned
            Reporter:
            Alex Karasulu
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development