Hi, Vladimir
On 10/8/19 3:18 PM, Vladimir Yaroslavskiy wrote:
The following renaming changes are important
and should be kept. My explanation is here
> ...
Thanks for the explanation - much appreciated.
* Other changes, like failed -> fail, i++ -> ++i,
and TypeConverter, FloatBuilder, DoubleBuilder, SortedBuilder
to be non-static can be reverted.
I can revert them, if you agree.
That would be great, thanks.
* I like that the various DualPivotQuickSort methods are tested
directly (via SortingHelper). I'd prefer that we also continue testing
direct calls to Arrays.sort(), as that's what users will call. What
would be the best way to add that back in? (Maybe a SortingHelper
variant that calls Arrays.sort()?).
I will add direct calls to Arrays.sort() and Arrays.parallelSort() also.
Great, thank you.
* Having the test operation depend on setting various values to the
static 'sortingHelper' field seems brittle to me. This could be a good
opportunity to convert from static to instance methods, make
'sortingHelper' and 'name' member variables, and create separate
Sorting
objects for each SortingHelper. Such a refactoring is not strictly
necessary, but I think it would be nice to have.
Good idea, I will implement it and send you update version very soon.
Sounds good!
test/jdk/java/util/Arrays/FailedFloat.java
Is this test necessary, given the addition of
testFloatNegativeZero() in
Sorting.java ?
Not necessary, because testFloatNegativeZero() covers this case.
This class was created to reproduce bug when float-pointing zeros
and NaNs are sorted.
Please remove FailedFloat.java. It makes sense to add the source of this
class to 8226297.
Done and done.
-Brent
