RFR: 8301492: Modernize equals() method of ResourceBundle.CacheKey and Bundles.CacheKey [v4]
Sergey Tsypanov
stsypanov at openjdk.org
Mon Jun 26 07:07:08 UTC 2023
On Sun, 25 Jun 2023 22:32:34 GMT, Pavel Rappo <prappo at openjdk.org> wrote:
>> I'll re-run our CI, and if all good, I'll sponsor this PR.
>
>> I'll re-run our CI, and if all good, I'll sponsor this PR.
>
> The CI tests I started have just passed. While this PR is already good, I wonder if we make it even better.
>
> I doubt highly that we need null-checks for CacheKey's name and locale. My suspicion comes from these two facts:
>
> * CacheKey's hashCode calls methods on these fields without checking them for null, and
> * CacheKey instances are queried and stored in ConcurrentHashMap, which certainly uses both `equals` and `hashCode`
>
> So, what I'm saying is that if those fields were null, there would be unhandled NPE somewhere; but I cannot see such NPE. Additionally, those fields are used in toString(). Furthermore, locale is known to not be null because the only place from where it is passed to a CacheKey consturctor is ResourceBundle.getBundleImpl(Module, Module, String, Locale, ResourceBundle.Control), which explicitly checks that locale for not being null.
>
> Does it make sense?
@pavelrappo you mean we can revert changes to ResourceBundle.java in lines 729 and 733?
-------------
PR Comment: https://git.openjdk.org/jdk/pull/12328#issuecomment-1606832885
More information about the core-libs-dev
mailing list