0
/* This program's aim is to count the number of bits set in an integer */

#include<stdio.h>

int count=0;
int check(int);

int main()
{
int i,r;
char ch;
printf("enter the integer for which you want to check the bits set");
scanf("%d", &i);
r=check(i);
printf("the number of occurance of 1 in the integer is  %d \n", r); /*don't know why isit         printing 0 when i give 4 as input */
    return 0;
}


int check(int j)
{

if((j & 1)==1)
count++;

for(int l=0;l<31;l++)
{

    if( j>>1 & 1)
    count++;

}
return count;

}

What is wrong with this program? Looks like some silly mistake or some conceptual one. Also do we need to write j>>1? can't we simply write j>>?

JIN007
  • 165
  • 12

1 Answers1

0

j >> 1 is wrong, so asking why you need to write that doesn't make much sense.

j >> makes even less sense. You don't write j + or j * either, do you? So why should that work for >>?

Anyway, you could test every bit by shifting by l (that's a lower case L, not a one), or you could change j so it only has to be shifted by 1 at every step. Here's a relatively minimal change to your code that does it the second way, I also changed the global count to a local because it was really poor form.

int check(int j)
{
    int count = 0;
    for (int l = 0; l < 32; l++)
    {
        if (j & 1)
            count++;
        j >>= 1;
    }
    return count;
}

There are still issues with it.

What if int is not 32 bits? What about the 32nd bit? (ok you can write l < 32 and maybe that's more in line with your code, I'm not sure what you really intended there)

Why is it even signed to begin with?

And check doesn't say much about what this function actually does, you could name this function popcnt or hammingweight or numberOfSetBits or something like that. And personally I'd use a different algorithm, but that's up to you.

harold
  • 61,398
  • 6
  • 86
  • 164
  • i am taking the integer to be 32 bit. To chec the once place I have used the first check on count and for the remaining 31 places I have used the loop. Is there any other flaw you see? – JIN007 Dec 11 '14 at 18:12
  • @user3287223 if you change the type to `uint32_t` and change the name, it's OK IMO. By the way if you're learning about bits and want some help in understanding it, you can drop by in #omnimaga on efnet if you want. (or ask more questions on stackoverflow, of course) – harold Dec 11 '14 at 18:26