2

I'm writing a program using bitwise operators in C. It's for a school assignment and the objective is to count the number of 1's in the bit representation of an integer, then print "Even" if the result is even and "Odd" if it's odd.

I wrote a program to loop through each bit and compare the inputted integer with a mask and increment a counter variable for each time the bitwise AND operator returns 1. However, the program doesn't seem to be incrementing the counter variable. Here's the code:

#include <stdio.h>

void bitsEvenOrOdd(int value);

int main(void) {
    int integer;

    printf("Enter an integer: ");
    scanf("%d", &integer);

    bitsEvenOrOdd(integer);

    return 0;
}

void bitsEvenOrOdd(int value) {
    unsigned int displayMask;
    unsigned int i;
    unsigned int counter;

    counter = 0;
    displayMask = 1 << 31;

    for (i = 0; i < 32; i++) {
        if ((value & displayMask) == 1) {
            counter++;
        }
        value <<= 1;
    }

    if ((counter % 2) == 0) {
        printf("The total number of 1's in the bit representation is even.\n");
    }
    else {
        printf("The total number of 1's in the bit representation is odd.\n");
    }
}

Any words of advice or tips are greatly appreciated. Thank you!

Federico klez Culloca
  • 26,308
  • 17
  • 56
  • 95
Sean
  • 2,890
  • 8
  • 36
  • 78
  • 1
    If you only have to print even/odd, you don't even have to count them, you can directly determine the [parity](http://stackoverflow.com/a/21618038/555045) – harold May 22 '17 at 11:22
  • 3
    Possible duplicate of [How to check if value has even parity of bits or odd?](http://stackoverflow.com/questions/21617970/how-to-check-if-value-has-even-parity-of-bits-or-odd) – harold May 22 '17 at 11:23
  • `int value` ==> `unsigned value`. Shifting a bit of `int` into the sign bit is undefined behaviour. – Weather Vane May 22 '17 at 11:23
  • [This SO post](http://stackoverflow.com/questions/44039795/why-does-this-function-count-the-number-of-set-bits-in-an-integer) should be useful for you. – LPs May 22 '17 at 11:25

4 Answers4

3

if ((value & displayMask) == 1) {

Consider this operation. displayMask is 0x80000000. Bitwise and between that number and any other number can only be 0x80000000 or 0. You're comparing it to 1.

Either compare if it's not equal to 0 or (I would recommend this), check the lowest bit set and shift right instead of left.

Art
  • 19,807
  • 1
  • 34
  • 60
0

You're shifting the wrong way:

value >>= 1;

Assuming an int is 32-bit, displayMask = 1 << 31 sets the highest order but in displayMask. When you left shift that value by 1, the single set bit gets shifted out, so your mask is now 0.

Change to a right shift.

value >>= 1;

Your comparison is also incorrect:

if ((value & displayMask) == 1) {

This will be true only if the mask is 1 and the low order bit is set. Instead, check if the result is nonzero:

if ((value & displayMask) != 0) {
dbush
  • 205,898
  • 23
  • 218
  • 273
0

Look at the line

if( (value & displayMask) == 1)

and consider what happens when displayMask is 2, 4, 8 etc

Malcolm McLean
  • 6,258
  • 1
  • 17
  • 18
0

You should be checking the lowest bit to fix the issue. Consider the below change in your code

         unsigned int counter;

         counter = 0;
    -    displayMask = 1 << 31;
    +    displayMask = 1;

         for (i = 0; i < 32; i++) {
         if ((value & displayMask) == 1) {
         counter++;
         }
     -        value <<= 1;
     +        value >>= 1;
     }

 if ((counter % 2) == 0) {