On Thu, 3 Oct 2024 21:02:16 GMT, Justin Lu <j...@openjdk.org> wrote: >> Please review this PR which improves the safety of equality checking for >> DecimalFormatSymbols. >> >> DecimalFormatSymbols via its setters, allows certain instance variables to >> be set as null. (Note that some variables are allowed to be null, and others >> are not.) However, non null safe comparisons are used for all variables >> during the equality check. This can lead to an unexpected NPE when >> `DecimalFormatSymbols.equals()` is invoked. >> >> The nullable variables in question should be equality checked via the static >> `Object.equals()`. An associated regression test is added to confirm the >> change. > > Justin Lu has updated the pull request incrementally with one additional > commit since the last revision: > > address Chen's comment - provide a NPE setter whitelist in test
I am not sure allowing null for those setters just because not to throw exceptions in equals() is the right solutoin. The distinction is made purely based on the existing behavior (which I believe is just an overlook in the first place). My preference is to null check in the setter methods, as there is no reason to set them null. That is just a misuse of the setters. ------------- PR Comment: https://git.openjdk.org/jdk/pull/21315#issuecomment-2400765289
