On Mon, 9 Nov 2020 18:38:33 GMT, Mark Sheppard <mshep...@openjdk.org> wrote:
>> I wonder if the spec should be a little more specific than just "seeded" >> which I think is fine for the first sentence. But maybe say something like >> "fields are copied from the given HttpRequest" in the description. > > a couple of points: > Should the spec state explicitly that an NPE is thrown if the supplied > HttpRequest reference is null? > > Alternatively, if the supplied HttpRequest is null then why not return an > empty builder? > > I would favour a slightly more verbose statement in the spec, something like: > > /** > * Creates a {@code Builder} with its pre request state set from that of > an existing {@code HttpRequest}. > * > * <p> This method can be used as the basis for building a new request > equivalent to a > * given request, while allowing further amendment and alteration of > * the pre request state, for example adding additional Http headers, > prior to request construction. > * > * @param request the request > * @return a new request builder > * @throws IllegalArgumentException if a new builder cannot be seeded from > * the given request (for instance, if the request contains > illegal > * parameters) > * @throws NullPointerException is thrown if the supplied HttpRequest is > null > * @since 16 > */ Hi Daniel, thanks for the clarification 👍 ... I missed that, as I only looked at the Builder and HttpRequest descriptions. So the second question does it seem reasonable to return an empty Builder if null is passed as an arg. However, if an NPE is thrown then it will force the client of the API to be more exact in its usage and avoid possible implicit ambuguities with a null reference args. regards Mark ------------- PR: https://git.openjdk.java.net/jdk/pull/1059