On Fri, 28 Feb 2025 18:44:14 GMT, Chen Liang <[email protected]> wrote:
>> I think this is confusing… If `CACHE.get(signature)` always returns a >> non-null value, there's no need for the condition `method != null` — it's >> always true. >> >> The javadoc for the `Cache` class specifies a `null` value can be returned: >> >> https://github.com/openjdk/jdk/blob/e98df71d9c5120fbb73a4c2f49863775fe5db781/src/java.desktop/share/classes/com/sun/beans/util/Cache.java#L98-L99 >> >> Is it a bug in `Cache` class that it creates the value and adds it to the >> cache when the value isn't found? >> >> https://github.com/openjdk/jdk/blob/e98df71d9c5120fbb73a4c2f49863775fe5db781/src/java.desktop/share/classes/com/sun/beans/util/Cache.java#L126-L127 > > Sorry for my last premature comment. > > I have looked at `Cache`, and believe `create` should be made protected to > indicate it is not intended to be called, but only overridden. > > The preexisting code called `create` to reuse the creation mechanism without > going through the cache - such usage is dubious. Per my experience, similar > conditional caches are better implemented by enclosing the logic into the > cache structure and use a common endpoint to access, so we have one universal > site to determine the conditionality of caching. In this case, such a > condition is better included in the `Cache` class itself, and the `create` > method should not be publicly exposed. Yes, I'd prefer `CACHE.get(signature)`. At the same time, I've got doubts that Alexander has… Although, strictly speaking, it is not “implementation dependent” because the `Cache` class is part of OpenJDK, and I think it's reasonable to depend on its implementation. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23845#discussion_r1975876473
