Update at http://codereview.appspot.com/157109/diff/4006/3003
2009/11/30 David-Sarah Hopwood <[email protected]>: > Mike Samuel wrote: >> 2009/11/29 David-Sarah Hopwood <[email protected]>: >>> David-Sarah Hopwood wrote: >>>> Mike Samuel wrote: >>>>> 2009/11/20 <[email protected]>: >>>>>> This is surprisingly tricky. I think the following is correct. >>>>>> >>> [...] >>>>>> public static long toInt32(double n) { >>>>>> double d = (n < 0.0 ? Math.ceil(n) : Math.floor(n)) % TwoToThe32; >>>>>> if (d < 0.0) d += TwoToThe32; >>>>> I'm confused by this step. >>>>> For n = -1, >>>>> d is initially -1, >>>>> then d gets changed to 0x7fffffff >>>>> but toInt32(-1) should be -1. >>>> No, it gets changed to 0xFFFFFFFF, which is cast by (int) to -1. >>> Gahh, no, I'm wrong. d gets changed to 0xFFFFFFFF, which is incorrectly >>> clamped to 0x7FFFFFFF by (int). >>> >>> Surprisingly tricky, indeed. To fix it, change toInt32 to use: >>> >>> return (int)(long) d; >> >> And similar changes could be made to toUint32 so that you end up with >> return ((long) d) & 0xffffffffL; >> ? > > The toUint32 code I gave was correct already, because it only cast to > (long). "return ((long) d) & 0xffffffffL;" would also be correct, though, > and removes a conditional. > > (Again, the comment would need fixing because d would not be > mathematically equal to int32bit in all cases.) > >>> If this change is made then "if (d < 0.0) d += TwoToThe32;" is no >>> longer needed (but the comment needs fixing if it is removed). > > -- > David-Sarah Hopwood ⚥ http://davidsarah.livejournal.com > >
