Question is Given - To Count Total number of set bits for 2 given numbers . For example take 2 and 3 as input . So 2 represents - 10 in binary form and 3 represents - 11 in binary form , So total number of set bits = 3.

My work -

#include <iostream>

using namespace std;

int bit(int n1, int n2){
    int count = 0;

    while(n1 != 0 && n2 != 0){
        if(n1 & 1 || n2 & 1) {
            count  ;
        }
        n1 >> 1;
        n2 >> 1;
    }
    
    return count;
}


int main() {
    
    int a;
    cin >> a;
    
    int b;
    cin >> b;
    
    cout << bit(a,b);
    
    return 0;
} 

Expected Output - 3

So please anyone knows what i am doing wrong please correct it and answer it.

CodePudding user response：

Why ask the question for 2 numbers if the intended combined result is just the sum of the separate results?

If you can use C 20, std::popcount gives you the number of set bits in one unsigned variable.

If you can't use C 20, there is std::bitset. Construct one from your number and use its method count.

So your code becomes:

int bit(int n1, int n2) {
#if defined(__cpp_lib_bitops)
    return std::popcount(static_cast<unsigned int>(n1))   std::popcount(static_cast<unsigned int>(n2));
#else
    return std::bitset<sizeof(n1)*8>(n1).count()   std::bitset<sizeof(n2)*8>(n2).count();
#endif
}

Demo

I'm not quite sure what happens if you give negative integers as input. I would do a check beforehand or just work with unsigned types from the beginning.

CodePudding user response：

It is possible without a loop, no if and only 1 variable.

First OR the values together

int value = n1 | n2; // get the combinedd bits

Then divide value in bitblocks

value = (value & 0x55555555)   ((value >> 1) & 0x55555555);
value = (value & 0x33333333)   ((value >> 1) & 0x33333333);
value = (value & 0x0f0f0f0f)   ((value >> 1) & 0x0f0f0f0f);
value = (value & 0x00ff00ff)   ((value >> 1) & 0x00ff00ff);
value = (value & 0x0000ffff)   ((value >> 1) & 0x0000ffff);

This works for 32bit values only, adjust fopr 64bit accordingly

CodePudding user response：

What you are doing wrong was shown in a (now deleted) answer by Ankit Kumar:

if(n1&1 || n2&1){
    count  ;
}

If a bit is set in both n1 and n2, you are including it only once, not twice.

What should you do, other than using C 20's std::popcount, is to write an algorithm¹ that calculates the number of bits set in a number and then call it twice.

Also note that you should use an unsigned type, to avoid implementation defined behavior of right shifting a (possibly) signed value.

---

^{1) I suppose that is the purpose of OP's exercise, everybody else shuold just use std::popcount.}