LANG-341: Add number to byte array methods to org.apache.commons.lang3.Conversion

[yurelle]

LANG-341: Add number to byte array methods to org.apache.commons.lang3.Conversion

• Add toPrimitive(byte[]) and toByteArray(primitive) functions
• Add hexToPrimitive(String) and toHex(primitive) functions
• Add bitString functions
• Add invertBitOrder() functions

[coveralls]

Coverage decreased (-0.05%) to 94.239% when pulling d55aa3ff4e2953d372b97023ce75af04ffb8ae1e on yurelle:LANG-341_AddNumToByteArrayMethods into e56ac1c37d236c9a0e4e7b86bd7d1b7201ac177d on apache:master.

[yurelle]

Crap. I used a primitive integer constant that's only in java 8. I'll fix.

[coveralls]

Coverage decreased (-0.003%) to 94.936% when pulling bd53cc4decb33872a9712bc9664477bb3de0e8d7 on yurelle:LANG-341_AddNumToByteArrayMethods into d1e9e598c9bcbf91afa174fa9b6c2ef30bbc8157 on apache:master.

[yurelle]

I got a little carried away; this PR is 4,500 lines. I'm sure this is probably going to take quite a while to validate and get accepted into the code base. I'm just glad I could get the ball rolling. If someone could check on my big/little endian implementations, I'd appreciate it. I can never keep those straight. I went off of these references: https://www.cs.umd.edu/class/sum2003/cmsc311/Notes/Data/endian.html https://en.wikipedia.org/wiki/Endianness https://en.wikipedia.org/wiki/Bit_numbering

I'm not sure why it's saying that test coverage went down. As far as I can tell, I've got tests for all my new functions.

One thing to note: in all my new functions that had a pre-existing alternate implementation, I added a chunk in the unit test to compare my results with the existing implementation. However, I had some trouble getting proper data out of the pre-existing hexToPrimitive() functions. The test code to do this validation is in the JUnit functions, but it's commented out. I'm probably just not understanding the existing implementation properly.

[dmjones]

Might be worth splitting this into smaller PRs that can be more easily reviewed?

The LANG-341 stuff was pretty much ready to go last time it was reviewed, so splitting that out would help get it into the code base sooner.

Some of the other methods are new (to me at least), and might warrant individual conversation - do we want/need these methods, etc. (Not saying we don't, but easier to have a coherent discussion about a smaller scope of change).

[dmjones]

Avoiding a mixture of white-space and code changes is also welcome :-)

[garydgregory]

Needs conflicts resolved...

[yurelle]

Ok, I fixed the merge conflicts and all the new build style tests that have been added since I first made this PR back in 2016. And the build works for Java 8, but it is dying on the Java 16 build. The errors are complaining about stuff that I didn't touch, so I don't think this is my doing.

Also, back when I wrote all this code, I wasn't allowed to use stuff that was added in Java 8 (specifically the BYTES static property that was added to the Primitive object classes). So, I manually created equivalent integer constants in the Conversion class. However, judging by the versions in the automated builds, it appears that java 8 is now the min supported version. If that is the case, then I can update this code to use the Java 8 BYTES fields, and remove the integer constants. Can you confirm if I'm allowed to use stuff that was introduced in Java 8?

Also, @dmjones mentioned a desire to have the PR broken up into chunks to make review easier. I can do that if you want, but I need to know how small you want those chunks to be. My original commit message split the changes into 4 groups of functions:

• primitive to byte[] and back
• hex to primitive and back
• bit strings
• invert bit order

Would these 4 PR's be sufficient? or do you want it more fine grained?

As for the whitespace, Intellij did it automatically; I'm not sure how to undo it.

-Yurelle

P.S. Sorry for leaving this in limbo for so long, but I've had depression for several years, and with a full-time job, I haven't had the energy to do much else. But I've had some time recently, and I'd like to finally get this finished, if yall still want it.

[garydgregory]

We know the build fails on Java 16, there is a separate thread about that on the dev ML. It looks like a problem in our data parser or possibly an issue in the JDK, hard to say, but we do need help figuring it out.

Gary

On Sun, Mar 28, 2021, 18:16 yurelle @.***> wrote:

Ok, I fixed the merge conflicts and all the new build style tests that have been added since I first made this PR back in 2016. And the build works for Java 8, but it is dying on the Java 16 build. The errors are complaining about stuff that I didn't touch, so I don't think this is my doing.

Also, back when I wrote all this code, I wasn't allowed to use stuff that was added in Java 8 (specifically the BYTES static property that was added to the Primitive object classes). So, I manually created equivalent integer constants in the Conversion class. However, judging by the versions in the automated builds, it appears that java 8 is now the min supported version. If that is the case, then I can update this code to use the Java 8 BYTES fields, and remove the integer constants. Can you confirm if I'm allowed to use stuff that was introduced in Java 8?

Also, @dmjones https://github.com/dmjones mentioned a desire to have the PR broken up into chunks to make review easier. I can do that if you want, but I need to know how small you want those chunks to be. My original commit message split the changes into 4 groups of functions:

• primitive to byte[] and back
• hex to primitive and back
• bit strings
• invert bit order

Would these 4 PR's be sufficient? or do you want it more fine grained?

As for the whitespace, Intellij did it automatically; I'm not sure how to undo it.

-Yurelle

P.S. Sorry for leaving this in limbo for so long, but I've had depression for several years, and with a full-time job, I haven't had the energy to do much else. But I've had some time recently, and I'd like to finally get this finished, if yall still want it.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/apache/commons-lang/pull/219#issuecomment-808968232, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJB6NZQ7CR5WU62E45545DTF6TETANCNFSM4CXUBPHQ .

[yurelle]

It looks like yall fixed the Java 16 build error. Sorry, I couldn't help with it. I tried looking into it, but couldn't figure it out.

[yurelle]

Oh, wait. Maybe not. I thought I saw the build badge saying was building successfully. sorry.

[XenoAmess]

@yurelle

throw new IllegalArgumentException("Input data is longer than size of 'double' primitive ("+SIZE+" bytes)");

tips: next time when you wanna bake some codes from a template for all primitive types, suggest use boolean, as boolean is the less troublesome one in them... if cannot use boolean then use double instead. do NEVER use int or long as template...that will cause the longer->doubleer thing.

[yurelle]

I never thought about using boolean; that's a good idea. Thanks.