Erm...



  • <font size="2"> unsigned long reverse(unsigned long theWord)
    {
        unsigned long result = 0;
        int i;
    </font>

    <font size="2">

        for (i = 0; i < 32; ++i) {

            if (theWord & (unsigned long) pow(2.0, (double) i))

                result += (unsigned long) pow(2.0, (double) (31 - i));

        }</font>

    <font size="2">

        return result;

    }</font>

    <font size="2">_____________________________________________________</font>

    <font size="2">This was borrowed from a "code database" website. Read. Need I say more?
    </font>



  • What's wrong with it? Seems like it'd work to me.



  • Since there is no way you can be serious, I don't think it's necessary
    to point out the wtfness of using expensive floating point arithmetic
    and math-lib functions instead of the much more efficient binary shift
    operations.



  • @akrotkov said:

    What's wrong with it? Seems like it'd work to me.

    On a system where an unsigned long is 32 bits, yes.  That isn't actually a universal constant, you know.



  • No matter what the integer size, this would work a whole lot better:

    <FONT face="Courier New"><FONT size=+0>unsigned long reverse(unsigned long theWord)
    {
        unsigned long result = 0;</FONT></FONT>

    <FONT face="Courier New"><FONT size=+0>    int i;</FONT> </FONT>

    <FONT size=+0>    for (i = 0; i < sizeof(theWord) * 8; ++i) {
            result <<= 1;
            result |= theWord & 0x01;
    </FONT><FONT size=+0>
            theWord >>= 1;</FONT><FONT size=+0>
        }</FONT>

    <FONT size=+0>    return result;
    <FONT face="Courier New">}</FONT></FONT>

     



  • I just want to make one more comment on this "brillant" piece of code. ;)

    This was apparently put into some production code. I told my friend about this code, and apparently he's seen this before at this company.
    /gasp
    /goggles



  • @kdean said:

    No matter what the integer size, this would work a whole lot better:

    <font face="Courier New"><font size="-0">unsigned long reverse(unsigned long theWord)
    {
        unsigned long result = 0;</font></font>

    <font face="Courier New"><font size="-0">    int i;</font> </font>

    <font size="-0">    for (i = 0; i < sizeof(theWord) * 8; ++i) {
            result <<= 1;
            result |= theWord & 0x01;
    </font><font size="-0">
            theWord >>= 1;</font><font size="-0">
        }</font>

    <font size="-0">    return result;
    <font face="Courier New">}</font></font>

     



    You misspelled "sizeof(theWord) * CHAR_BIT" out of the climits or limits.h header.


  • The fastest possible solution is to use lookup-tables with precomputed (partial) results.

    Two tables with 65536 entries each seem reasonable.


    unsigned int reverse4(unsigned int w)
    {
         return reverstable1[w&0xffff] | reversetable2[w>>16];

    }





  • unsigned long reverse(unsigned long i)
    {
       register unsigned long r=0;
       unsigned long m=((~0U)>>1)+1;
       for(register unsigned long b;i;i=b)r|=m/(i^(b=i&(i-1)));
       return r;
    }



  • @Angstrom said:

    @kdean said:

    No matter what the integer size, this would work a whole lot better:

    <FONT face="Courier New"><FONT size=+0>unsigned long reverse(unsigned long theWord)
    {
        unsigned long result = 0;</FONT></FONT>

    <FONT face="Courier New"><FONT size=+0>    int i;</FONT> </FONT>

    <FONT size=+0>    for (i = 0; i < sizeof(theWord) * 8; ++i) {
            result <<= 1;
            result |= theWord & 0x01;
    </FONT><FONT size=+0>
            theWord >>= 1;</FONT><FONT size=+0>
        }</FONT>

    <FONT size=+0>    return result;
    <FONT face="Courier New">}</FONT></FONT>

     



    You misspelled "sizeof(theWord) * CHAR_BIT" out of the climits or limits.h header.

    You can stop when theWord==0 instead of a bit count for more efficiency unless the >>= keeps the top bit.  Can't remember - long time since I did C.


Log in to reply