RFR: JDK-8323760 putIfAbsent documentation conflicts with itself [v2]
Pavel Rappo
prappo at openjdk.org
Wed Feb 14 15:13:04 UTC 2024
On Tue, 13 Feb 2024 17:11:05 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:
>> Update the documentation for `@return` tag of `putIfAbsent` to match the main description. `putIfAbsent` uses the same wording as `put` for its `@return` tag, but that is incorrect. `putIfAbsent` never returns the **previous** value, as the whole point of the method is not the replace the value if it was present. As such, if it returns a value, it is the **current** value, and in all other cases it will return `null`.
>
> John Hendrikx has updated the pull request incrementally with four additional commits since the last revision:
>
> - Add code block around argument
> - Reword
> - Reword to use put's previous value wording
> - Reword more clearly
Firstly, while it does not hurt to file CSR, in this case it feels like overkill: here you are fixing a glorified typo. It's a good and helpful fix, but it's also a typo fix. A good indication that CSR is not needed is that "Compatibility Kind" for this change is none of the available "source", "binary", or "behavioral".
Secondly, the current wording feels redundant and at the same time disjointed (bold font is mine):
> Returns null if the specified key was absent **or had a null value**, else returns the value currently associated with key. **(A null return can also indicate that the map previously associated null with the key, if the implementation supports null values.)**
Those parts I empathised are related, but they are also far apart and duplicate each other in a sense. We can get rid of the first part, retain the common wording in parentheses used in other `Map` methods, and swap clauses of the first sentence around:
> Returns the value currently associated with key, or null if the specified key was absent. (A null return can also indicate that the map previously associated null with the key, if the implementation supports null values.)
Don't rush to changing your PR, even if you like that proposal. Let's hear from others first.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/17438#issuecomment-1944028388
More information about the core-libs-dev
mailing list