RFR: 8247402: Documentation for Map::compute contains confusing implementation requirements

[Pavel Rappo]

On Wed, 25 Nov 2020 06:19:04 GMT, Stuart Marks <smarks at openjdk.org> wrote:

>>> The proposed CSR has a few problems that we need to resolve.
>>> 
>>> 1. The **Specification** pseudo-code behaves differently from both the old pseudo-code and the actual implementation when `newValue == null && oldValue == null` and `map.containsKey(key) == true`.
>>> 2. The content of the **Solution** section seems irrelevant: aside from a couple of missing `return` statements the current pseudo-code is fine. We are after something else, aren't we? The bottom line is we should state the solution more clearly.
>>> 3. The **Summary** section differs from that of the JDK-8247402.
>> 
>> I read my previous reply and realized that it is confusing and contains a factual error; so let me straighten it out in this new reply rather than edit that previous one.
>> 
>> 1. The proposed pseudo-code behaves exactly the same way as the existing pseudo-code modulo the missing `return` statements. (For some reason, I previously wrote that the proposed pseudo-code behaves differently from the existing pseudo-code.)
>> 2. Both the proposed pseudo-code and the existing pseudo-code deviate from the documented behaviour (written in prose) and the actual implementation. The deviation happens when `newValue == null && (oldValue = map.get(key)) == null` and `map.containsKey(key) == true`. (That part was correct.)
>> 
>> Now, here's what I should have said in my previous reply. If the CSR intends to solve (2) then both the proposed pseudo-code and the **Problem** section must be updated; otherwise the **Solution** section must be updated. Put differently, either fix the diff and add one more item to that problem list, or change the solution. Otherwise the solution does not match the problem leaving the CSR in a contradictory state.
>> 
>> I hope this all makes sense now.
>
>> 2. Both the proposed pseudo-code and the existing pseudo-code deviate from the documented behaviour (written in prose) and the actual implementation.
> 
> Let me clarify something. The "documented behavior" is actually the API specification, the contract that applies to Map.compute and all its implementations. The pseudo-code is part of the "implementation requirements" section, whose primary responsibility is to specify what the default implementation actually does. (Of course, that implementation should also conform to the API spec.)
> 
> We've established that the pseudo-code -- either the existing or proposed version -- differs from the default implementation's actual behavior. It's thus failed its primary responsibility, and therefore that's the problem that needs to be fixed. I think it's pointless to clean up the pseudo-code if the result is still incorrect.

@stuart-marks I thought about it before reading your reply. And since I hadn't come to a definite conclusion, I worded my response to @johnlinp as a choice.

Although in this particular case it's easy to fix the discrepancy, I think it raises a general question which applies to all (pseudo-) code snippets in normative parts of API specifications:

**What is the required level of fidelity particular (pseudo-) code has to have?**

It's such a huge topic for discussion, that I don't even know where to begin; certainly, not in this PR thread. Perhaps we should start a dedicated (non-RFR/PR) thread on one of the OpenJDK mailing lists to host that discussion.

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

PR: https://git.openjdk.java.net/jdk/pull/714