<i18n dev> RFR: 8301492: Modernize equals() method of ResourceBundle.CacheKey and Bundles.CacheKey [v4]

[Pavel Rappo]

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?

Yes, those lines. We can revert to x.equals(otherEntry.x).

-------------

PR Comment: https://git.openjdk.org/jdk/pull/12328#issuecomment-1607062302