[jdom-interest] SAXBuilder setFeature() and setProperty() exceptions

[Jason Hunter]

Elliotte Rusty Harold wrote:
> 
> As indicated in the JavaDoc for SAXBuilder setFeature() and setProperty():
> 
> NOTE: SAXBuilder requires that some particular features of the SAX
> parser be set up in certain ways for it to work properly. The list of
> such features may change in the future. Therefore, the use of this
> method may cause parsing to break, and even if it doesn't break anything
> today it might break parsing in a future JDOM version, because what JDOM
> parsers require may change over time. Use with caution.
> 
> That's an important note. I wonder if we shouldn't also enforce it in
> code? That is, would it be a good idea to have  setFeature() and
> setProperty() throw an exception (probably something like
>  BuilderConfigrationException extends JDOMException)  if someone passes
> a known conflicting value for  such a feature; e.g. tries to turn
> namespace processing off.

Well, if we throw an exception for some features/properties, that will
make people think that if we don't throw an exception they're not at
risk.  It doesn't even benefit people that much since they'll soon
enough realize they have a problem when the document is wrongly created.

My gut tells me it's best to pass through everything and let them see
the results than to try to help them but only do so partially and not
necessarily reliably.

> The SAX equivalents of these methods in XMLReader are declared to throw
> SAXNotRecognizedException
> <http://java.sun.com/j2se/1.4/docs/api/org/xml/sax/SAXNotRecognizedException.html>
> and SAXNotSupportedException
> <http://java.sun.com/j2se/1.4/docs/api/org/xml/sax/SAXNotSupportedException.html>
> 
> Along a similar note, we're delaying those exceptions until a document
> is actually built.  I'd prefer them to be thrown immediately  until the
> unrecognized;/unsupported feature/property is set. I think the reason we
> delay them is to allow the parser to be swapped out after the builder is
>  initially constrcuted, but is this really something that's needed?
> Currently, that functionality is only available from  the protected
> interface. I don't see anyway to access from the public interface. Is it
> really so much to ask that the client create a new instance of
> SAXBuilder if they want to use a different parser?

The model has been to create and configure the parser during the build
call and not on SAXBuilder construction.  That allows subclasses to have
some flexibility by hooking into the process at varying points.  

It probably came from the early decision to re-create parsers before
each build, something that's not so necessary these days and has been
discussed being redone.

I'd like to hear what other people think.  I believe it provides a tiny
bit of niceness to the API but at the expense of breaking lots of
existing code (throwing exceptions where there used to be none).  While
I know we're free to do that, I don't like breaking people's code unless
we can show them a significant improvement in the API.  I'm not sure
this is that significant.  I'd like to hear what others think.

-jh-


-jh-