On Fri, 22 Jan 2021 14:48:00 GMT, Daniel Fuchs <dfuchs@openjdk.org> wrote:

...

Tighten up argument checking in constructor.

src/java.base/share/classes/java/time/zone/ZoneRules.java line 263:

...

261: // last rules 262: Object[] temp = lastRules.toArray(); 263: ZoneOffsetTransitionRule[] rulesArray = Arrays.copyOf(temp, temp.length, ZoneOffsetTransitionRule[].class);

LGTM. Could be replaced by:

ZoneOffsetTransitionRule[] rulesArray = (ZoneOffsetTransitionRule[])lastRules.toArray(new ZoneOffsetTransitionRule[0]).clone();

if you wanted - but what you currently have is good for me.

Or even maybe `rulesArray = lastRules.toArray(ZoneOffsetTransitionRule[]::new);`? ------------- PR: https://git.openjdk.java.net/jdk/pull/2191

On Fri, 22 Jan 2021 17:12:34 GMT, Daniel Fuchs <dfuchs@openjdk.org> wrote:

...

Or even maybe `rulesArray = lastRules.toArray(ZoneOffsetTransitionRule[]::new);`?

Good point - but that would be:

ZoneOffsetTransitionRule[] rulesArray = lastRules.toArray(ZoneOffsetTransitionRule[]::new).clone();

Interesting. This last one is more concise, but it's a bit harder to reason about. The lastRules implementation could return an array of a type other than ZOTR[]. If it's unrelated or a supertype, this would result in ClassCastException -- probably not a problem. If it were a subtype of ZOTR[], this would get stored in the object's field. Is this a problem? Turns out it can't happen, since ZOTR is final. While not wrong, I don't think this is the right idiom. It occurs to me that there should by another overload Arrays.copyOf(array, newType) that changes the type without changing the length. This would let us get rid of the local variable. ------------- PR: https://git.openjdk.java.net/jdk/pull/2191