On Tue, 5 Apr 2022 23:16:57 GMT, liach <d...@openjdk.java.net> wrote:
>> XenoAmess has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> revert changes in jdk.compile
>
> src/java.base/share/classes/java/util/LinkedHashMap.java line 804:
>
>> 802: * @since 19
>> 803: */
>> 804: public static <K, V> LinkedHashMap<K, V> newLinkedHashMap(int
>> numMappings) {
>
> `LinkedHashMap` may be often extended for it has a `protected boolean
> removeEldestEntry(Entry)`. Should we make a separate factory method for such
> instances (with functional implementation) or just expose
> `HashMap.calculateHashMapCapacity`?
Good question. Having to subclass and override this method in order to provide
a removal policy has always seemed rather clumsy. However, it's the supported
approach, and it's done fairly frequently in the wild. A new subclass requires
that the caller invoke `new` on that specific subclass, which in turn must
choose which superclass constructor to call. This means that a static factory
method can't be used. The alternatives would be to expose another constructor
or to expose `calculateHashMapCapacity` as you suggest. A new constructor might
also need to control the load factor and the ordering policy (insertion vs
access order) so that's a fairly complicated overload to consider.
Exposing the calculate method might help but that's mostly just a wrapper
around a small computation. As we've seen it's easy to get this computation
wrong, but exposing a method that _just_ does this computation seems like a
really narrow case.
(Still another alternative would be to pass a lambda expression that's called
at the appropriate time. That would involve adding a `BiPredicate<Map<K,V>,
Map.Entry<K,V>>` to yet another constructor overload. This could work but it
doesn't seem any simpler.)
The need for subclassing LinkedHashMap and overriding this method might also be
reduced by the addition of new APIs from the Sequenced Collections proposal
(https://openjdk.java.net/jeps/8280836). One simply needs to call
`pollFirstEntry` at the right time. That might remove the need to have some
expiration policy plugged directly into the map itself.
I'm not inclined to add more APIs to cover what seems to be a fairly narrow
case, but we might keep this in mind to see if anything useful pops up.
-------------
PR: https://git.openjdk.java.net/jdk/pull/7928
