feat: Hamming Code for 32bit values - #1431
Conversation
Alternative with parity bit check.
Test suite for 32-bit values in different formats.
Added hamming code with parity value checks
chrdek
left a comment
There was a problem hiding this comment.
Changed some code in HammingCode.test.js
Sorry, something went wrong.
chrdek
left a comment
There was a problem hiding this comment.
.
Sorry, something went wrong.
appgurueu
left a comment
There was a problem hiding this comment.
- It's not clear what you're trying to do here besides "hamming code". What is your dictionary (what are your code words, what do they decode to?). What is the hamming distance of your code words? Add useful comments, use better variable names.
- The code is currently a mess. Please reformat it.
- Don't use var, use let or const instead.
- The tests should use each.
Sorry, something went wrong.
Refactored tests, usage of .each for most test cases.
Changes according to specs. - more understandable variable names/ comments etc. - usage of let/const Note: code refers to bit population count of a binary sequence
There was a problem hiding this comment.
added:
more comments
understandable variable names, more info.
Tests: Refactored , now all vitest tests utilize each.
Sorry, something went wrong.
There was a problem hiding this comment.
Please implement my first review: Format the code, make it clear what you're doing and why.
This is a lookup-table accelerated software popcount. What does this have to do with hamming codes?
The unrelated triangular number change also does not belong here.
We also technically already have BinaryCountSetBits.js for this, but its implementation is pretty naive and I'd like to see it replaced.
Sorry, something went wrong.
Changes in name of algorithm.
Additional tests and other name changes.
There was a problem hiding this comment.
As per initial request:
- Included let,const in the codebase
- Added more comments in-code
- Changed the name to Hamming weight since it is the pop. count's more appropriate naming.
- This algorithm can be easily extended for larger values as well.
And Remember: "JS code is like beer, under the same brand type there are different flavors and variations based on ingredients and fermentation process." Happy micro-brewing on your repo! cheers. 🍺🙃
Sorry, something went wrong.
Reviewers
Requested changes must be addressed to merge this pull request.
Describe your change:
Checklist:
Example:UserProfile.js is allowed but userprofile.js,Userprofile.js,user-Profile.js,userProfile.js are not