Changes are incorporated and the updated binary is attached. I'm sorry
to hasten you but I want them to be fixed as soon as possible.
> 1. Since its datatypeLibrary not dataTypelibrary, the names of classes
> should use Datatype not DataType.
>
> 2. I don't think Datatype.check() needs to be optional. If implementations
> can't provide detailed information about why a match fails they can just
> throw an uninformayive DatatypeException.
>
> 3. The protected members of DataTypeException are evil. Make them private
> and final and add accessors.
>
> 4. Add a constructor with no arguments to DataTypeException to deal with 2.
>
> 5. I don't think the content member of DataTypeException is necessary. The
> caller knows this already.
Done.
> 6. displayName on DataType is not necessary. The caller knows what the name
> of the datatype is, since they used the name to create it with
> createDataTypeBuilder or getType. Conceptually, the datatype library is a
> mapping from local names to datatypes. The name isn't part of the datatype.
> The object representing a <data> pattern should store the name in addition
> to the DataType object.
I still have a sentiment to it, but probably you are right.
> 7. Return value of characters on DataTypeStreamingValidator is unclear. For
> example, if we have a streaming validator for int, and the first call to
> characters passes "+", what does characters() return? It should return true
> because it no error has been detected. isValid() should be called only when
> you have finished accumulating the value. Thus characters() does not return
> what isValid would return after processing the fragment. For this reason, I
> think isValid() should be renamed to something like finish() or end().
Done.
> 8. How does DatatypeStreamingValidator provide the diagnosis information
> that you get from the DatatypeException? This should be possible, index in
> DatatypeException should be an index into the entire string.
Added the check method. Any suggestion for a better name?
> 9. Get rid of reset() on DatatypeStreamingValidator. It's an
> over-optimization and complicates the semantics of the object.
I modified the semantics of the DatatypeStreamingValidator, and allow
the finish method and the check method to be called only once. This
should give the datatype library a chance to reuse the streaming
validator objects, if that is necessary.
> 10. On ValidationContext, I think if we support isUnparsedEntity(), we
> should also support isNotation(). (SAX supports both.)
Done. But no known datatype library uses this method.
> 11. We need to think about how instances of DatatypeLibrary get created.
> Ideally, what we want is that an implementor of a DatatypeLibrary provides
Added a DatatypeLibraryFactory class to the helper package. Since this
class relies on a non-standard class "sun.misc.Service", I think it's
not a bad idea to make it an optional "utility" class.
This class has protected "create" method, which should be implemented by
the datatype library to create a DatatypeLibrary object from an URI.
It also has the find method, which enumerates available DatatypeFactory
implementations and call its create method to see if it supports the
specified URI.
> 12. DataType needs a valueHashCode() method consistent with sameValue() in
> order to implement keys.
Done. I guess a custom implementation of Set would also help...
regards,
--
Kohsuke KAWAGUCHI +1 650 786 0721
Sun Microsystems
