>>>>> On Wed, 18 Nov 2015 23:01:50 +0000 (UTC), <pbenedict_at_apache.org> said:
PB> My initial impression of PushBuilder:
PB> * I find it a surprising choice to make it a default method that always
PB> returns an instance. This is very confusing, IMO, because the
PB> connection may not support the feature -- which clearly begs the
PB> question why (1) instantiate anything and (2) return an object that
PB> does nothing?
PB> * The docs talk about "If the current connection does not support
PB> server push", but there is no way to query for that fact.
>>>>> On Thu, 19 Nov 2015 13:47:34 +1100, Greg Wilkins <gregw_at_webtide.com> said:
GW> I believe that feedback has already been received that there should
GW> be an isPushSupported method and then either null or ISE can be
GW> returned if it is not. I'm OK with either approach.
Hi Greg, by "either approach" do you mean "null or ISE" or the currently
specified no-op approach. Note that I have fixed the erroneously
introduced NoOpPushBuilder.
PB> * Is it wise for a spec to provide server implementation classes? I
PB> don't think this precedent should be set. That was clearly the purpose
PB> for introducing NoOpPushBuilder.
That was a mistake and it has bee removed:
8<----
r64223 | edburns | 2015-10-29 21:29:30 -0400 (Thu, 29 Oct 2015) | 10 lines
https://java.net/jira/browse/SERVLET_SPEC-134
SECTION: Modified Files
A src/main/java/javax/servlet/http/NoOpPushBuilder.java
M src/main/java/javax/servlet/http/HttpServletRequest.java
- Correct incorrectly public implementation detail class.
8<----
Sorry about that.
GW> Hmmm a bit late for the servlet spec to avoid implementation classes :)
GW> But the approach above avoids the need for the noop.
When Greg says "a bit late" he is talking about much older things than
the erroneously public NoOpPushBuilder.
Personally, I like the no-op-approach (without having the NoOpPushBuilder
in the API) because it allows frameworks not worry about whether or not
push is supported.
PB> * The methods like "etag" and "path" are truly setters, not operations.
PB> I think prefixing them with "set" should be considered... and returning
PB> "void" to keep the bean spec.
Greg, what do you think? I like it as is. It is more consistent with
how builders are done.
Paul, you are encountering what is, in my opinion, a fundamental
incompatibility between the fluent API style and the JavaBeans API
style. My general guidance on API design when confronting this
incompatibility is to not mix the two. That is, once you go into
"builder API" mode for a feature, stick with that style completely for
the breadth and depth of that feature.
>>>>> On Thu, 19 Nov 2015 10:01:25 -0600, Paul Benedict <pbenedict_at_apache.org> said:
PB> Thanks Greg for your response. As this was my first time writing, I
PB> didn't know if anyone from the EG reads the user list -- and I don't
PB> think I can mail comments directly to the EG list anyway.
That is correct, and I respect you sticking with the process.
PB> As for the builder pattern, I don't think switching styles makes
PB> sense with the history of the servlet spec API. It's kind of
PB> changing horses in mid-stream. I would prefer a consistent style
PB> throughout *unless* the class is labeled as "xxxBuilder". Such a
PB> design makes sense, for example, with JAX-RS UriBuilder but unless
PB> the class says itself is a builder, it's going to lead to confusion,
PB> IMO.
The class is named PushBuilder. Please help me understand your concern,
I must be missing something.
Thanks,
Ed
--
| edward.burns_at_oracle.com | office: +1 407 458 0017