On May 26, 2016, at 3:49 PM, Stuart Marks <stuart.ma...@oracle.com> wrote: > > > > On 5/26/16 2:28 PM, Mandy Chung wrote: >>> - return h.keySet(); >>> + return Set.of(h.keySet().toArray(new String[0])); >>> } >> >> The patch looks fine. It’d be good to add a test case. >> >> If you use Collections.unmodifiableSet, you would not need to convert the >> key sets to an array. Any benefit of using Set::of instead of >> Collections.unmodifiableSet? > > There are several minor tradeoffs. > > Collections.unmodifiableSet() simply creates and returns a wrapper object. It > contains the keySet, which is backed by the HashMap created within the > stringPropertyNames() method. This keeps references to all the string keys > and values around, even though the values aren't used. But the strings > themselves are also referenced from the original Properties object and its > default chain. > > The toArray() call copies the key string refs to an array, then Set.of() > hashes them into its internal array. But the resulting data structure holds > only references to the keys, so it's more compact. > > Given that most uses seem to iterate the set's elements and then throw it > away, it's probably not worth the creation overhead of for an immutable Set. > Drat. I guess I was too eager to use the new API. :-) > > I've changed this to use Collections.unmodifiableSet(); see below. >
Looks fine. I don’t have any objection in using Set::of vs Collections.unmodifiableSet() in this case (Sorry. I should have made it clear). No need for new patch if you decide to use Set.of (a comment might help). Mandy
