1
\$\begingroup\$

I'm writing a program which determines how many bits are in a floating point value (this will be run on Unix).

#include <stdio.h>
#include <stdlib.h>

void countbits(int k);

int main(){
    int input;
    // int a = (int) malloc(sizeof(int));
    printf("Please enter a number: ");
    scanf("%d",&input);
    // a = input;

    countbits(input);
}

void countbits(int k){
    int j,count;
    count = 0;
    for(j = 0; j < 31; j++){
        if(k & 0x000001 == 1){
        count++;
        }
        k >>= 1;
    }
    printf("%d has %d bits set\n",k,count);
    // count = 0;
}

Output example:

2 has 1 bits set
6 has 2 bits set
32 has 1 bits set

I'll really appreciate any feedback or correction or if there is something I can do better.

\$\endgroup\$

closed as unclear what you're asking by 200_success May 5 '14 at 9:51

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ I don't see how this code has anything to do with floating-point. Please explain? I'm putting the question back on hold in the meantime. \$\endgroup\$ – 200_success May 5 '14 at 9:49
2
\$\begingroup\$

You should do the output in main(), not in countbits(). Functions should have only one primary purpose, according to the Single Responsibility Principle (SRP). In this case, countbits() should just perform the calculations and return the result. Thus, its return type should now be of type int.

Inside of countbits(), you could just loop while k is not 0 instead of looping 32 times. Once all the bits in k shift away and k becomes 0, there's no point in continuing. This will also give you a result of 0 right away if the input is 0; calculating this will be a waste of time.

int main(void) {
    int input;

    // get the input...

    int count = countbits(input);

    printf("%d has %d bits set\n", input, count);
}

int countbits(int k) {
    int count = 0;

    while (k != 0) {
        // perform calculations...
    }

    return count;
}

Some notes about this and other things:

  • main() should have a void parameter since it isn't taking any command line arguments.
  • The first (unformatted) output in main() should use puts() instead of printf().
  • count doesn't need to be declared and then assigned to 0. Just initialize it to 0.
  • j should be initialized inside the for loop statement (if you have C99). If you don't, then declare it right before the loop. Variables should be used as close in scope as possible.
\$\endgroup\$
  • \$\begingroup\$ Thanks Jamal! Can you talk little bit about my method in countbits. I'm not so sure about it if its correct. \$\endgroup\$ – user3478869 May 5 '14 at 4:31
  • \$\begingroup\$ Professor gave us example what our program should output he has 4 prints 2. 32 prints 2. 16 prints 3. 2 prints 1. However my 32, 4, 16, 2 all prints 1. \$\endgroup\$ – user3478869 May 5 '14 at 4:34
  • \$\begingroup\$ @user3478869: Since this isn't working as it should, I will have to place the question on hold until it works. We only review working code (and I had assumed it was tested for correctness first). You can ask Stack Overflow on how to fix this. After it works, edit in the changes (but do not apply my changes), and the hold can be removed. \$\endgroup\$ – Jamal May 5 '14 at 4:43
  • \$\begingroup\$ It has been confirmed that it is working correctly. \$\endgroup\$ – user3478869 May 5 '14 at 4:53
  • \$\begingroup\$ @user3478869: With the current code, or with some changes? My test code is also showing different results from the expected output. \$\endgroup\$ – Jamal May 5 '14 at 4:59

Not the answer you're looking for? Browse other questions tagged or ask your own question.