close
Skip to content

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

Open
yurelle wants to merge 6 commits intoapache:masterfrom
yurelle:LANG-341_AddNumToByteArrayMethods
Open

LANG-341: Add number to byte array methods to org.apache.commons.lang3.Conversion#219
yurelle wants to merge 6 commits intoapache:masterfrom
yurelle:LANG-341_AddNumToByteArrayMethods

Conversation

@yurelle
Copy link
Copy Markdown

@yurelle yurelle commented Nov 26, 2016

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
Copy link
Copy Markdown

Coverage Status

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

@yurelle
Copy link
Copy Markdown
Author

yurelle commented Nov 26, 2016

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

…3.Conversion

- Add toPrimitive(byte[]) and toByteArray(primitive) functions
- Add hexToPrimitive(String) and toHex(primitive) functions
- Add bitString functions
- Add invertBitOrder() functions
@yurelle yurelle force-pushed the LANG-341_AddNumToByteArrayMethods branch from d55aa3f to 41103d1 Compare November 26, 2016 23:03
@coveralls
Copy link
Copy Markdown

coveralls commented Nov 26, 2016

Coverage Status

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

@yurelle
Copy link
Copy Markdown
Author

yurelle commented Nov 26, 2016

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
Copy link
Copy Markdown
Contributor

dmjones commented Dec 4, 2016

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
Copy link
Copy Markdown
Contributor

dmjones commented Dec 4, 2016

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

@garydgregory
Copy link
Copy Markdown
Member

Needs conflicts resolved...

@yurelle yurelle force-pushed the LANG-341_AddNumToByteArrayMethods branch from 436bd9e to 3f95eff Compare March 28, 2021 21:14
- Fix whitespace build errors.
@yurelle yurelle force-pushed the LANG-341_AddNumToByteArrayMethods branch from 3f95eff to d59ab1b Compare March 28, 2021 21:23
error for hex lookup maps.
@yurelle yurelle force-pushed the LANG-341_AddNumToByteArrayMethods branch from 951d7ab to fee1f6f Compare March 28, 2021 21:43
@yurelle
Copy link
Copy Markdown
Author

yurelle commented Mar 28, 2021

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
Copy link
Copy Markdown
Member

garydgregory commented Mar 29, 2021 via email

@yurelle
Copy link
Copy Markdown
Author

yurelle commented Jun 10, 2021

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
Copy link
Copy Markdown
Author

yurelle commented Jun 10, 2021

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

@XenoAmess
Copy link
Copy Markdown
Contributor

XenoAmess commented Jun 14, 2021

@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
Copy link
Copy Markdown
Author

yurelle commented Jun 14, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants