On 07/08/14 17:26 , sebb wrote:
Again, looks generally OK.

Some comments below.

On 6 August 2014 10:29,  <[email protected]> wrote:
+@Immutable

Are we sure it's immutable?

+final class DistinguishedNameParser {
+
+    public final static DistinguishedNameParser INSTANCE = new 
DistinguishedNameParser();
+
+    private static final BitSet EQUAL_OR_COMMA_OR_PLUS      = 
TokenParser.INIT_BITSET('=', ',', '+');
+    private static final BitSet COMMA_OR_PLUS               = 
TokenParser.INIT_BITSET(',', '+');

BitSet is not immutable, so using a shared static field relies on the
code never calling any of the mutation methods after creation.


True, but these static variables are private and their state does not mutate post construction.

Perhaps consider using a simple delegation wrapper to provide only read access?
Otherwise we need to document the non-mutability assumption.


We could but given those static variables are private I see no real need to do so.

+
+    static class InternalTokenParser extends TokenParser {
+

Do we really need this?
Perhaps escape processing should be merged into the super-class.


Yes, we do. HTTP spec states that escaped chars are valid only when enclosed in double quotes. It is quite different from RFC 4514 and earlier RFCs superseded by it.


But if it is kept, it needs to have Javadoc.


It is a package private class. Does this make any sense?

Oleg


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to