Skip to content

Added javadoc messages concerning hash64 seed and sign extension.#34

Merged
garydgregory merged 1 commit into
apache:masterfrom
Claudenw:CODEC-275_Update_javadoc
Dec 28, 2019
Merged

Added javadoc messages concerning hash64 seed and sign extension.#34
garydgregory merged 1 commit into
apache:masterfrom
Claudenw:CODEC-275_Update_javadoc

Conversation

@Claudenw

Copy link
Copy Markdown
Contributor

Fix for CODEC-275

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 93.321% when pulling ee89227 on Claudenw:CODEC-275_Update_javadoc into e4de423 on apache:master.

@aherbert

Copy link
Copy Markdown
Contributor

I am 50/50 on this.

I would not call it a bug when there is no reference hash64 implementation. This method is a port of a bad adaption of the hash128x86 method to output a 64-bit hash. So unlike the other method where it was a bug because the output was not the same as the reference implementation, this can be labelled as an implementation detail.

The only real effect is that the initial hash will be xor'd with 1's instead of 0's for the upper 32-bits and if the seed is negative.

Since we do not know about the statistical properties of this hash, it is not part of the original code and it is marked as deprecated then adding a lot of javadoc about sign-extension bugs which do not manifest (because the default seed is positive) to 5 deprecated methods seems like noise to me.

I would update the javadoc on the main method which accepts a seed to state that the seed is not treated as unsigned.

@garydgregory

Copy link
Copy Markdown
Member

I am going to bring this in because it helps understand the code more, especially since these comments are all on deprecated methods.

@garydgregory garydgregory merged commit e918527 into apache:master Dec 28, 2019
@garydgregory

Copy link
Copy Markdown
Member

asfgit pushed a commit that referenced this pull request Dec 28, 2019
@Claudenw Claudenw deleted the CODEC-275_Update_javadoc branch December 28, 2019 16:28
omosteven pushed a commit to omosteven/commons-codec-lab-work that referenced this pull request Jan 8, 2025
omosteven pushed a commit to omosteven/commons-codec-lab-work that referenced this pull request Jan 8, 2025
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.

4 participants