Re: Request for comments: Bug 6306820

2007-06-28 Thread Michael McMahon

Richard Kennard wrote:

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 think I agree with respect to the names of the methods, but I think the
comment (from Mark) was only to do with the return type of the 
multi-valued getXXX().
He is saying that it should be a List instead of a String[]. I 
think I agree with
him. There may be a conflict here between the desire to fit in with the 
Java SE API conventions
and the desire to remain "familiar" with existing Java EE APIs, but the 
CCC's policy will

be to stick with the Java SE conventions.

Thanks
Michael.


Re: Request for comments: Bug 6306820

2007-06-28 Thread Michael McMahon

Some other minor comments on the last draft.
- Michael.

Class intro docs.

1. In the section entitled "Multivalued parameters" the list refers to 
getParameterValue()

   This should be getParameter()

2. The first reference to "(ampersand and semicolon)" should inlcude the 
actual symbols

   themselves for clarity eg. (ampersand "&" and semicolon ";")

3. I'd remove the section about URL. There is no need to mention it at all.

4. Section on Thread Safety. Change "not safe for use by multiple 
threads" to just

   "not synchronized".

Other comments on the method definitions

5. If the use of [] arrays is being changed to List<>, then any of the 
methods that take

   these as parameters: create() , getParameterMap().

6. Why does the parse() method say that calling it with an invalid 
string, is undefined?

   Would it not be better to throw an exception with an invalid string?
Same comment applies to other methods with similar spec.

7. The notes in italics referring to servlet should be removed I think.

8. I think the docs for the 2nd setParameter() variant should say 
something different
   from the 1st one. Maybe an example showing how a literal or 
primitive integer type

   would be accepted as an argument.