Javadoc conventions in the presence of default methods

[Joe Bowbeer]

I don't think a proposal like this would be necessary except for the
inheritance aspect.  Otherwise, some conventions such as "the default
implementation" versus "this implementation" would suffice.

I'd like to see more examples, including a few layers of subclasses, where
the doc contains both implspec and implnote.  The Input/OutputStream
classes and Swing Component classes are a good source for material.



On Thu, Jan 31, 2013 at 6:31 PM, Brian Goetz <brian.goetz at oracle.com> wrote:

> We've tried this thread a few times without success, so let's try it again.
>
> There have been a number of cases where its not obvious how to document
> default methods.  After some analysis, this appears to be just another case
> where the complexity was present in Java from day 1, and default methods
> simply bring it to the fore because the confusing cases are expected to
> come up more often.  The following applies equally well to methods in
> abstract classes (or concrete classes) as to defaults.
>
> There are lots of things we might want to document about a method in an
> API.  Historically we've framed them as either being "specification" (e.g.,
> necessary postconditions) or "implementation notes" (e.g., hints that give
> the user an idea what's going on under the hood.)  But really, there are
> four boxes (and we've been cramming them into two):
>
>   { API, implementation } x { specification, notes }
>
> (We sometimes use the terms normative/informative to describe the
> difference between specification/notes.)
>
> As background, here are some example uses for default methods which vary
> in their "expected prevalence of overriding".  I think the variety of use
> cases here have contributed to the confusion on how to document
> implementation characteristics.  (Note that all of these have analogues in
> abstract classes too, one can find examples in Abstract{List,Map,Set}.)
>
> 1.  Optional methods.  This is when the default implementation is barely
> conformant, such as the following from Iterator:
>
>     public default void remove() {
>         throw new UnsupportedOperationException(**"remove");
>     }
>
> It adheres to its contract, because the contract is explicitly weak, but
> any class that cares about removal will definitely want to override it.
>
> 2.  Methods with *reasonable* defaults but which might well be overridden
> by implementations that care enough.  For example, again from Iterator:
>
>     default void forEach(Consumer<? super E> consumer) {
>         while (hasNext())
>             consumer.accept(next());
>     }
>
> This implementation is perfectly fine for most implementations, but some
> classes (e.g., ArrayList) might have the chance to do better, if their
> maintainers are sufficiently motivated to do so.  The new methods on Map
> (e.g., putIfAbsent) are also in this bucket.
>
> 3.  Methods where its pretty unlikely anyone will ever override them, such
> as this method from Predicate:
>
>     public default Predicate<T> and(Predicate<? super T> p) {
>         Objects.requireNonNull(p);
>         return (T t) -> test(t) && p.test(t);
>     }
>
> These are all common enough cases.  The primary reason that the Javadoc
> needs to provide some information about the implementation, separate from
> the API specification, is so that those who would extend these classes or
> interfaces can know which methods they need to / want to override.  It
> should be clear from the doc that anyone who implements Iterator MUST
> implement remove() if they want removal to happen, CAN override forEach if
> they think it will result in better performance, and almost certainly don't
> need to override Predicate.and().
>
>
> The question is made more complicated by the prevalent use of the
> ambiguous phrase "this implementation."  We often use "this implementation"
> to describe both normative and informative aspects of the implementation,
> and readers are left to guess which.  (Does "this implementation" mean all
> versions of Oracle's JDK forever?  The current version in Oracle's JDK?
>  All versions of all JDKs?  The implementation in a specific class?  Could
> IBM's JDK throw a different exception from UOE from the default of
> Iterator.remove()?  What happens when the doc is @inheritDoc'ed into a
> subclass that overrides the method?  Etc.  The phrase is too vague to be
> useful, and this vagueness has been the subject of many bug report.)
>
> I think one measure of success of this effort should be "can we replace
> all uses of 'this implementation' with something that is more informative
> and fits neatly within the model."
>
>
> As said earlier, there are four boxes.  Here are some descriptions of what
> belongs in each box.
>
> 1.  API specification.  This is the one we know and love; a description
> that applies equally to all valid implementations of the method, including
> preconditions, postconditions, etc.
>
> 2.  API notes.  Commentary, rationale, or examples pertaining to the API.
>
> 3.  Implementation specification.  This is where we say what it means to
> be a valid default implementation (or an overrideable implementation in a
> class), such as "throws UOE."  Similarly this is where we'd describe what
> the default for putIfAbsent does.  It is from this box that the
> would-be-implementer gets enough information to make a sensible decision as
> to whether or not to override.
>
> 4.  Implementation notes.  Informative notes about the implementation,
> such as performance characteristics that are specific to the implementation
> in this class in this JDK in this version, and might change.  These things
> are allowed to vary across platforms, vendors and versions.
>
> Once we recognize that these are the four boxes, I think everything gets
> simpler.
>
>
> Strawman Proposal
> -----------------
>
> As a strawman proposal, here's one way to explicitly label the four boxes:
> add three new Javadoc tags, @apinote, @implspec, and @implnote. (The
> remaining box, API Spec, needs no new tag, since that's how Javadoc is used
> already.)  @impl{spec,note} can apply equally well to a concrete method in
> a class or a default method in an interface.
>
> (Rule of engagement: bikeshedding the names will be interpreted as a
> waiver to ever say anything about the model or the semantics.  So you may
> bikeshed, but it must be your last word on the topic.)
>
> /**
>   * ... API specifications ...
>   *
>   * @apinote
>   * ... API notes ...
>   *
>   * @implspec
>   * ... implementation specification ...
>   *
>   * @implnote
>   * ... implementation notes ...
>   *
>   * @param ...
>   * @return ...
>   */
>
> Applying this to some existing Javadoc, take AbstractMap.putAll:
>
>     Copies all of the mappings from the specified map to this map
>     (optional operation). The effect of this call is equivalent to
>     that of calling put(k, v) on this map once for each mapping from
>     key k to value v in the specified map. The behavior of this
>     operation is undefined if the specified map is modified while
>     the operation is in progress.
>
>     This implementation iterates over the specified map's
>     entrySet() collection, and calls this map's put operation
>     once for each entry returned by the iteration.
>
> The first paragraph is API specification and the second is implementation
> *specification*, as users expect the implementation in AbstractMap,
> regardless of version or vendor, to behave this way.  The change here would
> be to replace "This implementation" with @implspec, and the ambiguity over
> "this implementation" goes away.
>
> The doc for Iterator.remove could be:
>
>  /**
>   * Removes from the underlying collection the last element returned by
>   * this iterator (optional operation). This method can be called only
>   * once per call to next(). The behavior of an iterator is unspecified
>   * if the underlying collection is modified while the iteration is in
>   * progress in any way other than by calling this method.
>   *
>   * @implspec
>   * The default implementation must throw UnsupportedOperationException.
>   *
>   * @implnote
>   * For purposes of efficiency, the same UnsupportedOperationException
>   * instance is always thrown. [*]
>   */
>
> [*] We don't really intend to implement it this way; this is just an
> example of an @implnote.
>
>
> The doc for Map.putIfAbsent could be:
>
>   /**
>    * If the specified key is not already associated with a value,
> associates
>    * it with the given value.
>    *
>    * @implspec
>    * Th default behaves as if:
>    * <pre> {@code
>    * if (!map.containsKey(key))
>    *   return map.put(key, value);
>    * else
>    *   return map.get(key);
>    * } </pre>
>    *
>    * @implnote
>    * This default implementation is implemented essentially as described
>    * in the API note. This operation is not atomic. Atomicity, if desired,
>    * must be provided by a subclass implementation.
>    */
>
>
> Secondary: one can build on this to eliminate some common inheritance
> anomalies by making these inheritable separately, where @inheritDoc is
> interpreted as "inherit the stuff from the corresponding section."  This is
> backward compatible because these sections do not yet exist in old docs.
>  SO to inherit API spec and implementation spec, you would do:
>
>  /**
>   * {@inheritDoc}
>   * @implspec
>   * {@inheritDoc}
>   * ...
>   */
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/lambda-libs-spec-experts/attachments/20130131/7bca5d2a/attachment-0001.html