Re: Request for comments: Bug 6306820
The CCC request has received initial approval, which simply means that the API change is accepted in principle. But, we need to finalize the specification and submit it to the CCC for final approval. Due to the (large) amount of discussion that has already taken place, I don't anticipate that this will take too long. The following is a comment from the final CCC reviewer. The comments are based on an older version of the API, but they are still applicable to the current version. - Michael A couple of quick comments: - The names of the elements of the URIQueryString.ParameterSeparator enum should be in UPPER_CASE, per convention. (Actually, defining an enum for this seems like overkill to me; I'd be tempted to accept a string argument and throw an IllegalArgumentException for anything except "&" and ";"). - Using string arrays in the URIQueryString API is just wrong; these should be List. (Mimicing the ServletRequest API is of marginal value at best, and so not an argument for using String[].)
Re: Request for comments: Bug 6306820
Michael, Given that I sort of disagree with both these comments, should I just take it that this is what the CCC wants and make the changes anyway? My disagreements would be: 1. Overkill or not, surely defining an enum is more explicit, more type-safe and a generally stricter way of doing things that defining the contract in the JavaDoc and throwing an IllegalArgumentException at runtime? 2. I think the whole 'a URL parameter can have multiple values' issue is unintuitive, and returning either a List or a String[] is going to be a surprise to people who just want a single String return value (witness the confusing nature of getParameter and getParameterValues in the Servlet API). Therefore, I'd rather follow ServletRequest which has beaten down this path for many years. But, as always, I'll defer to the CCC if that's what you'd prefer? Richard. Michael McMahon wrote: The CCC request has received initial approval, which simply means that the API change is accepted in principle. But, we need to finalize the specification and submit it to the CCC for final approval. Due to the (large) amount of discussion that has already taken place, I don't anticipate that this will take too long. The following is a comment from the final CCC reviewer. The comments are based on an older version of the API, but they are still applicable to the current version. - Michael A couple of quick comments: - The names of the elements of the URIQueryString.ParameterSeparator enum should be in UPPER_CASE, per convention. (Actually, defining an enum for this seems like overkill to me; I'd be tempted to accept a string argument and throw an IllegalArgumentException for anything except "&" and ";"). - Using string arrays in the URIQueryString API is just wrong; these should be List. (Mimicing the ServletRequest API is of marginal value at best, and so not an argument for using String[].)
Re: Request for comments: Bug 6306820
Richard Kennard wrote: Michael, Given that I sort of disagree with both these comments, should I just take it that this is what the CCC wants and make the changes anyway? My disagreements would be: 1. Overkill or not, surely defining an enum is more explicit, more type-safe and a generally stricter way of doing things that defining the contract in the JavaDoc and throwing an IllegalArgumentException at runtime? I think I agree since it does need to specified in the apply() method which separator you are talking about. Though I think we should stick with the uppercase convention. 2. I think the whole 'a URL parameter can have multiple values' issue is unintuitive, and returning either a List or a String[] is going to be a surprise to people who just want a single String return value (witness the confusing nature of getParameter and getParameterValues in the Servlet API). Therefore, I'd rather follow ServletRequest which has beaten down this path for many years. What are you suggesting here? Michael.
Re: Request for comments: Bug 6306820
Michael,
Though I think we should stick with the uppercase convention.
Agreed.
What are you suggesting here?
Nothing different to what I have been saying before. HTML requires us to
have the 'multi-valued return' feature, but I think if the API looks like...
class UrlEncodedQueryString
{
void append( String name, String value );
void set( String name, String value );
String get( String name );
List getValues( String name );
}
...people are going to be 'Huh? What's the difference between get() and
getValues()? When I put() or add() Strings in a List or a Map I don't
need a get() and a getValues(), why do I need them here?'. Whereas, if
the API looks like...
class UrlEncodedQueryString
{
void appendParameter( String name, String value );
void setParameter( String name, String value );
String getParameter( String name );
String[] getParameterValues( String name );
}
...it feels a little more familiar. People can say 'hey this is weird,
but it's the same kind of weird I'm used to from the Servlet API'.
I keep going back to the Servlet API because it's the only Java SE/EE
API, as far as I know, that directly addresses this same area. However,
if I'm the only one that feels this way I'll go with the former?
Richard.
