Good to see this happen.
I agree a test would be good to demonstrate edge cases.
1)
I think the code for floorMod(long x, int y); cannot actually
overflow. As such, the result could just be cast without the if and
throw.
2)
My preferred algorithm for floorMod is:
return ((a % b) + b) % b;
as it contains no Java-side branches, although tests would be needed
to prove performance.
This also allows an algorithm for floorDiv with no Java-side branches:
int mod = ((a % b) + b) % b;
return (a - mod) / b;
3)
While making changes, this code could be changed to avoid the local
variable (just return):
public static int floorMod(int x, int y) {
int r = x - floorDiv(x, y) * y;
return r;
}
Stephen
On 29 September 2015 at 03:17, Joseph D. Darcy <joe.da...@oracle.com> wrote:
> Hi Brian,
>
> Do you think any tests are needed here, at least for a quick sanity check?
>
> Thanks,
>
> -Joe
>
>
> On 9/28/2015 6:06 PM, Brian Burkhalter wrote:
>>
>> Please review at your convenience.
>>
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8023217
>> Patch: http://cr.openjdk.java.net/~bpb/8023217/webrev.00/
>>
>> Summary:
>> Add some additional methods of the same name as existing methods but with
>> the commonly used parameter signature “long,int”:
>>
>> long multiplyExact(long,int)
>> long floorDiv(long,int)
>> int floorDiv(long,int)
>>
>> These methods also provide hooks for potential intrinsics. There might be
>> room for improvement in their Java implementations.
>>
>> The modifications to the java.time classes are to fix warnings about
>> “redundant cast to int.”
>>
>> Thanks,
>>
>> Brian
>
>
