Add Kernighan's Algorithm for Counting Set Bits#1107
Add Kernighan's Algorithm for Counting Set Bits#1107smitgabani wants to merge 10 commits intoTheAlgorithms:masterfrom
Conversation
appgurueu
left a comment
There was a problem hiding this comment.
Please fix your code style.
|
There are still unresolved review comments. |
| @@ -0,0 +1,22 @@ | |||
| import { CountSetBits } from '../CountSetBits' | |||
There was a problem hiding this comment.
I was more thinking something like:
test("CountSetBits", () => {
const tests = {
// binary representation: set bits
'1001': 2,
'111': 3,
}
for (binary in tests)
expect(CountSetBits(parseInt(binary, 2))).toBe(tests[binary])
})There was a problem hiding this comment.
I did not intend a reversal to the more verbose form. You can even use a list of lists if you like - all you need is a convenient way to test that a certain input leads to a certain output without copy-pasting. Here's another alternative:
const testCountSetBits = (input, expected_output) => expect(CountSetBits(input)).toBe(expected_output)
test('CountSetBits', () => {
testCountSetBits(25, 3)
testCountSetBits(36, 2)
})etc., or, IMO preferable since it makes the binary representation apparent:
const testCountSetBits = (input_bin_str, expected_output) => expect(CountSetBits(parseInt(input, 2))).toBe(expected_output)
test('CountSetBits', () => {
testCountSetBits("0", 0)
testCountSetBits("10", 1)
})etc.
| expect(res).toBe(0) | ||
| test('CountSetBits', () => { | ||
| const tests = { | ||
| // binary representation: set bits |
There was a problem hiding this comment.
This is not accurate anymore - now this is a decimal representation. I also don't quite like this, since it relies on back-and-forth coercion of numbers to strings and back to numbers when passing them to CountSetBits.
|
|
||
| test('CountSetBits', () => { | ||
| const tests = { | ||
| // binary representation: set bits |
There was a problem hiding this comment.
The algorithm accepts decimal representation and counts the bits in its binary representation. The comment binary representation:set bits in not correct and I will remove it in the next commit.
There was a problem hiding this comment.
I have changed the comment it should be a decimal representation to set bits. Counting the number of set bits in a binary representation is only counting the number of ones and the algorithm does not do that. It counts bit sets in the decimal representation of the number. The tests and algorithm deal with numbers only and not strings.
There was a problem hiding this comment.
https://iq.opengenus.org/brian-kernighan-algorithm/
The algorithm accepts an integer as given in the algorithm.
There was a problem hiding this comment.
It counts bit sets in the decimal representation of the number.
A decimal representation does not have a notion of bits. A decimal representation has digits from 0 - 9.
The tests and algorithm deal with numbers only and not strings.
I'm aware. But using numbers as keys for a JS object will coerce them to strings. When passing the numbers into CountSetBits, JS will coerce them back to numbers as soon as you start doing arithmetic & bit ops on them.
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |


Describe your change:
Checklist:
Example:
UserProfile.jsis allowed butuserprofile.js,Userprofile.js,user-Profile.js,userProfile.jsare notFixes: #{$ISSUE_NO}.