Christopher Hegarty - Sun Microsystems Ireland wrote:
Questions (Sorry, of this has come up before):
1) Why have overloaded versions of toString and parse. Why not one
method called setSeparator?
We need to support both separator types in the same input strings.
Though I thought that
the original separators had to be preserved, but it seems this is not
the case.
So, it might be possible to allow both in input strings but use a
setSeparator()
method to determine which to use for output.
2) Does getParameterNames return multiple entries for multiple name
value pairs. I think not.
No, it shouldn't.
3) Do we really need removeParameter. Isn't this the same as
setParameter(name, null)?
Yes, it is the same. We could remove removeParameter.
comments:
I'll let Richard comment on the following ones.
Class Description
1) "This class provides constructors for creating..."
This should be 'methods' or static/factory methods.
I think you should probably also inline links to the methods. I
presume they are create & parse.
2) I would make ", methods for creating, retrieving, updating and..." a
new sentence. "Methods for creating,..."
3) I think that the code samples in the class description should follow
the java coding conventions.
http://java.sun.com/docs/codeconv/html/CodeConventions.doc7.html#682
For example, the following blank spaces should be removed:
URI uri = new URI( "http://java.sun.com?forum=2"; );
UrlEncodedQueryString queryString = new UrlEncodedQueryString( uri );
System.out.println( queryString.getParameter( "forum" ));
URI uri = new URI("http://java.sun.com?forum=2";);
UrlEncodedQueryString queryString = new UrlEncodedQueryString(uri);
System.out.println(queryString.getParameter("forum"));
Similar for other code examples.
Method description:
1) Both parse methods start with "This constructor...". -> method.
2) getParameterValues. returns a List not an array. The method
description also needs to be updated.
3) getParameterMap. Same as 2)
4) How does setParameters(java.lang.String) handle Separators?
5) ditto for appendParameters.
6) apply(URI, UrlEncodedQueryString.Separator). 'Separator' in the
parameter list should start with lowercase 's'.
-Chris.
Michael McMahon wrote:
I have updated the CCC request for this, and would like
to finalize it this week.
Can I get comments on it please?
The apidocs can be seen at
http://oldsunweb.ireland/~mm72272/urlencodedquerystring/
Thanks
Michael.
