On 4 October 2016 at 23:27, Stuart Marks <stuart.ma...@oracle.com> wrote: > 4) The 'resolverFields' problem/comment was regarding DateTimeFormatter (see > this part of latest webrev), where I realised I couldn't use Set.of instead > of Collections.unmodifiableSet(new HashSet<>(Arrays.asList(*))), because I > noticed that one of the java.time tests was testing whether > DateTimeFormatter.withResolverFields(TemporalField...) could accept null > parameters, which made using Set.of impossible since it's null-hostile (as > in it throws NullPointerException upon being constructed with null > element(s)). > > Hm, yes, it's odd that there's a test for an array containing a null, in > addition to an empty array and a null array. See: > > jdk/test/java/time/tck/java/time/format/TCKDateTimeFormatter.java > test_resolverFields_listOfOneNull() > > I'm not entirely sure what's intended here. In any case, let's wait until we > hear from Mr. Colebourne.
I don't think that is a sensible test. AFAICT, null in the set has no meaning. Its not documented to have meaning, so it is really a bug with a test. > 5) For the changes in the Chronology classes, the era() method now returns > an immutable array where it didn't before. (The List returned by > Arrays.asList() can have individual elements modified but its size can't be > changed.) The spec for eras() says "may be immutable" so presumably this is > OK. But note, since the returned List is now immutable, a new instance > needn't be created each time. I'm not sure whether it's worth keeping around > a cached copy in case someone calls eras() again though. > > It would be good if we could hear from Stephen on this one as well. eras() will rarely be used, so no point caching. Changing to List.of() is fine. Stephen
