Request for comments: Bug 6306820

[Alan Bateman]

Richard Kennard wrote:
> Alan,
>
> I read some distinct 'themes' into your last e-mail. Please permit me 
> to respond to the 'themes', rather than your exact points:
>
> 1. java.net.URL
>
> You said:
>
> 'I would suggest ignoring java.net.URL completely... is parse(URL) 
> needed? ... is apply(URL) needed? ... I believe naming conventions 
> would suggest this be should Url* instead'
>
> I actually encounter java.net.URL quite often in my daily work - many 
> APIs seem to use it, and don't seem to offer a java.net.URI 
> alternative? I think it is still quite active and shouldn't be ignored?
You've pulled comments from different contexts here but no matter. The 
URL class has many problems and unfortunately they cannot be fixed for 
compatibility reasons. This is why the spec has a recommendation to use 
URI over URL. There are toURL and toURI methods to convert between the 
classes. We should encourage developers to use URI for new code (and I 
agree there is a large body of existing code that uses URL).

For the static factory methods that construct the object by parsing an 
existing query string then you a number of choices. The proposed 
parse(CharSequence) looks good to me. If there is code using 
java.net.URL then it can be simply invoked using parse(url.getQuery()) 
and I don't see a problem with that. There is more argument for a 
parse(URI) method as developers can get confused as to if the query 
component, in string form, should be decoded or not.

The "apply" methods could benefit from some discussion. As I mentioned, 
it may be worth seeing if we should have more general methods to 
construct URIs by combining an existing URI and other components. 
Michael may have views on this.

As regards the name of the class, then Josh Bloch's Effective Java book 
has a good section on this. To follow those guidelines the class would 
be named UrlEncodedQueryString rather than URLEncodedQueryString. I just 
mention it as something to think about (and yes, we have lots of naming 
inconsistencies in the platform).

>
> 2. Servlet API
>
> You said:
>
> 'it's not clear to me why any of the method names need the word 
> "Parameter" in them... It's not clear to me that we have to be 
> consistent with the Servlet API.'
>
> I think consistency with the Servlet API is important because it is 
> the only example in the JDK (well, Java EE DK) of an API that deals 
> specifically with www-form-urlencoded query strings: it is mature and 
> very familiar to many developers?
>
> It also has some naming quirks around returning String versus String[] 
> for getParameter and getParameterValues. Those quirks are the 
> necessary because of the HTML spec, so I'd rather match the Servlet 
> API's naming quirks than introduce a brand of quirks of our own?
My only concern with copying the method names from the Servlet spec is 
that it makes it awkward when constructing one of these objects by 
chaining a sequence of method invocations. If the method names are 
shorter then the syntax can be neater, eg:
  UrlQueryString qs = UrlQueryString().create().set(n1,v1).set(n2,v2);
works well in my mind.

Whether the map value is String[] or List<String> isn't a major issue. I 
just suggested thinking about List<String> as it may be more useful in 
the long term.

>
> 3. Separators
>
> You said:
>
> 'Is this enum needed? ... maybe the create method can specify the 
> separator? ... what if [the query string to be parsed] uses a 
> different separator? ... Do you have examples in mind where a query 
> would be parsed using one separator but then converted to use a 
> different separator?
>
> I'm afraid I thought I'd explained this already? It's also in the 
> JavaDoc:
>
> "ALL separators are recognised when parsing query strings. ONE 
> separator may be passed to 'getQuery' and 'apply' when outputting 
> query strings"
Ah, those words are in the spec for the ParameterSeparator enum and so 
didn't appear in the javadoc that you appended.  In that case, I would 
suggest making this clear in the spec for the parse spec. Should the 
parse spec also say anything about the non-default separators - if they 
can appear in values then I assume they must be escaped prior to parsing

>
> I agree on your point around removing 'getQuery' and just having 
> 'toString'.
Did you decide on the method to construct a string with the non-default 
separator? Naming it toString(seperator) would be consistent.

-Alan.