Re: Datatype interface for RELAX NG (updated)

[James Clark <jjc@jclark.com>]

One additional comment.  I don't think DatatypeException needs the
Datatype parameter.  The caller can always get this information from
elsewhere.  Just an index and a message should be sufficient.

> > 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?

We do need a better name.  check() and finish() don't really make sense
as a pair.   And there needs to be consistency between the
names/semantics of the methods on Datatype and the names/semantics of
the corresponding methods on DatatypeStreamingValidator.

Also I think it's necessary to be able to call finish and then call
check on the same object. (Imagine having multiple
DatatypeStreamingValidators operating in parallel; only when all of them
return false for finish, do you want an exception thrown.)

How about this?

Rename Datatype.allows to isValid.
Rename DatatypeStreamingValidator.finish to isValid
Rename Datatype.check to checkValid.
Rename DatatypeStreamingValidator.check to checkValid

Semantics of DatatypeStreamingValidator.isValid are that it returns true
if the accumulated literal is a valid lexical representation of the
type.  However, it does not conceptually change the state of the
DatatypeStreamingValidator object. Thus you can call it multiple times
and it will return the same value. (Generally I think its cleaner if
functions that return values don't mutate state.)  Similarly checkValid
will have exactly the same semantics as isValid except that it throws an
exception in cases where isValid would return false.

It might be clearer to rename characters() to something like
addCharacters().

> > 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.

You can't use internal undocumented Sun classes in public code!

We have to do the work that sun.misc.Service does in the helper class,
ie call

ClassLoader.getResources("META-INF/services/org.relaxng...")

and parse the files to get the resulting class names and then
instantiate the classes.

> 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.

The class that defines the service provider interface isn't a helper
class; it's part of the interface to the datatype library.  The helper
class should just deal with creating an instance of service interface.
This helper can probably go away when Sun gets round to including a
standard way to instantiate service providers in the Java platform
(which is badly needed -- there are separate implementations in both IIO
and JAI at the moment).  There also may need to be different versions of
the helper because ClassLoader.getResources isn't in JDK 1.1.x, so JDK
1.1.x based systems will only be able to use a single datatype library,
or may need a different strategy.

Summary:

- Move DatatypeLibraryFactory into the org.relaxng.datatype. It should
just have a create method.

- Create a DatatypeLibraryRegistry helper class that just has a find
method. It should be non-static, so that it can cache the list of
DatatypeLibraryFactory objects.  Probably you should also be able to
specify the ClassLoader to use for finding the resources.  If you don't
want to write this class, then leave it out; but don't include something
using Sun internal classes in the package.

(Alternatively, instead of adding a new DatatypeLibraryFactory class to
org.relaxng.datatype, add an additional method to DatatypeLibrary
returning the URI of the library; then you could use the name
DatatypeLibraryFactory for the helper class.)

James