HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Print Bits Part II

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
bitspartprint

Problem

Could you suggest ways of making this more simple?

/*
 * Next write a function invert(x,p,n) that returns x with the n bits
 * that begin at position p inverted, leaving the others unchange
 */

void printbits(unsigned x) {
    size_t size_of_int = sizeof(int) >= 1) {
       ((x & mask) == 0) ? printf("0") : printf("1");
       ((i & 3)==0) ? printf(" ") : printf("%s","");
    }
    printf("\n");
}

void invert(unsigned x, unsigned p, unsigned n) {
    printbits(((~((((~(~0 << n) << p))) & x)) & (~(~0 << n) << p)) | (x & ~(~(~0 << n) << p)));
}

int main(int argc, char *argv[]) {
    unsigned input=3082676239, begin=15, nbits=5;
    invert(input, begin, nbits);
    return(0);
}


Before mushing the parts together this is what I get in output:

x = 1111 0000 0001 0111 1011 1101 1110 1101 
===================================================================
          mask0 = 1111 0000 0001 0110 0000 1101 1110 1101 
          mask1 = 1111 1111 1111 1110 0100 1111 1111 1111 
          mask2 = 0000 0000 0000 0001 1111 0000 0000 0000 
===================================================================
         output = 1111 0000 0001 0110 0100 1101 1110 1101

Solution

@palacsint's definitely provides an improvement, but I think we can do better still. I think I'd start from the fact that subtracting 1 from a number clears the least significant bit that was set, and sets all the less significant bits. If we start with a number that has only one bit set, that bit will be cleared, and all the less significant bits will be set. Based on that, getting a mask of N bits is pretty simple: take 1, shift it left N bits, and then subtract 1.

For example, consider creating a 5-bit mask in a 16-bit number:

0000 0000 0000 0001    // 1
0000 0000 0010 0000    // 1 << 5
0000 0000 0001 1111    // (1<<5)-1


Once we have that, we can shift it left p bits to get it to the right position, and xor with the input:

unsigned invert(unsigned x, unsigned p, unsigned n) { 
    unsigned mask = ((1u << n) - 1u) << p;
    return x ^ mask;
}


A couple minor points:

  • I've removed printing from invert -- IMO, printing the result should be separate.



  • This requires that `n



I think the printing can be simplified a bit as well. Although the "test for a multiple of 4" inside the loop does work, I think at least in this case a nested loop makes the intent clearer:

void print(unsigned x) { 
    static const int group_size = 4;
    int group, j;

    for (group = 0; group >= 1)
            printf("%c", (x & 1) + '0');
        printf(" ");
    }
}


Another possibility that might be worth considering would be a small lookup table to convert 4 bits at a time:

void print(unsigned x) { 
    static const int group_size = 4;
    // inverted order because you're printing the LSB first.
    static const char *outputs[] = {
        "0000", "1000", "0100", "1100", "0010", "1010", "0110", "1110", 
        "0001", "1001", "0101", "1101", "0011", "1011", "0111", "1111"
    };
    int group;

    for (group=0; group>= 4)
        printf("%s ", outputs[x & 0xf]);
}

Code Snippets

0000 0000 0000 0001    // 1
0000 0000 0010 0000    // 1 << 5
0000 0000 0001 1111    // (1<<5)-1
unsigned invert(unsigned x, unsigned p, unsigned n) { 
    unsigned mask = ((1u << n) - 1u) << p;
    return x ^ mask;
}
void print(unsigned x) { 
    static const int group_size = 4;
    int group, j;

    for (group = 0; group < size_of_int / group_size; group++) {
        for (j=0; j<group_size; j++, x >>= 1)
            printf("%c", (x & 1) + '0');
        printf(" ");
    }
}
void print(unsigned x) { 
    static const int group_size = 4;
    // inverted order because you're printing the LSB first.
    static const char *outputs[] = {
        "0000", "1000", "0100", "1100", "0010", "1010", "0110", "1110", 
        "0001", "1001", "0101", "1101", "0011", "1011", "0111", "1111"
    };
    int group;

    for (group=0; group<size_of_int / group_size; group++, x >>= 4)
        printf("%s ", outputs[x & 0xf]);
}

Context

StackExchange Code Review Q#6125, answer score: 5

Revisions (0)

No revisions yet.