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.