On Tue, 13 Feb 2024 17:11:05 GMT, John Hendrikx <jhendr...@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

Reply via email to