Thanks Roger!

On 12/21/2015 1:35 PM, Roger Riggs wrote:
Hi Joe,

A few comments:

InputSource:

I assume you meant javax.xml.transform.Source (there was an error in Source's javadoc where the return statement referred to it as InputSource).

- Should the default method return true? The default method is only present to allow source compatibility with unknown subtypes. I would expect that without
   any specific implementation knowledge it would need to be false.
  All of the JDK provided Sources will implement isEmpty appropriately.

Changed to throw UOE now as Joe recommended.


- Editorial: "Empty means that there is no input available from this Source".

Fixed.

- from the definition and implementation it seems that isEmpty will be true when a stream (streamSource) is positioned just before EOF. Is that intended?

Yes. In general, a Source object shall be passed to a processor with input positioned at the beginning. If for some reason it's not, the Source object will not handle the situation.


stream/StreamSource:
- Does not need to check both sources; if the first source has any input, return false immediately.

Fixed.

 - the choice to return false when an IOException occurs seems odd;
if reading throws an exception then it seems unlikely that any future read could work;
   and hence there is no more input.

It's true no future read would work. But since a Source object can be used in various situations where "empty" and failure to read are treated differently by the processor, the isEmpty method needs to abort its process, return false to indicate it's not empty, thus allow the processor to handle the error.

I added a statement to the spec.


- Note also that you probably need separate try/catch handlers for the stream vs the reader. An exception from reading the inputStream should no preempt the reader.

As discussed above, in case of error, isEmpty will abort its process and return false.


sax/InputSource:
  - ditto the comments above.

In the test:
 - This test is duplicated:

+ "{new SAXSource(new InputSource(new StringReader("")))},
+  {new SAXSource(new InputSource(new StringReader("")))},

Fixed.

http://cr.openjdk.java.net/~joehw/jdk9/8144967/webrev/

Thanks,
Joe



Roger


On 12/17/2015 7:25 PM, huizhe wang wrote:
Hi,

Adding a convenient method isEmpty to javax.xml.transform.Source and org.xml.sax.InputSource.

JBS: https://bugs.openjdk.java.net/browse/JDK-8144967
webrev: http://cr.openjdk.java.net/~joehw/jdk9/8144967/webrev/

Thanks,
Joe



Reply via email to