Conversation
| return values.iterator(); | ||
| } | ||
| }; | ||
| return List.copyOf(values); |
There was a problem hiding this comment.
Would this impose any regression on performance? Returning an iterator (view) looks cheaper than taking a copy of the values.
There was a problem hiding this comment.
@Fokko That's probably true, but in practice are EnumTypes ever large enough to matter? I don't know.
What about returning Collections.unmodifiableList(values) ? I believe that would still return a view but be more defensive about preventing modification of the underlying data.
There was a problem hiding this comment.
We could also make private final List<EnumValue> values; a unmodifiableList from the start and just directly return that in getValues(). Although maybe that's technically a breaking change if someone was relying on being able to indirectly update the list.
Or the simplest change to keep current behavior but express it in a Java 11 idiom would be return () -> values.iterator();
I'm ok with any of these options!
There was a problem hiding this comment.
Update this to just return values::iterator since that's in keeping with the spirit of the PR (java modernization). Other changes could be discussed separately. Thanks @Fokko
Now that java 8 is no longer supported, we can use language features introduced in later versions and get ahead of other changes for subsequent upgrades. I'm trying this in one module as a test of automating this with codex. Of the four points below the first seems like it should definitely be addressed. The others are potentially useful, but not pressing.
Codex generated summary: