[Yanel-dev] XMLHelper added

[Alec Bickerton]

Guillaume Déflache wrote:
> Michael Wechner schrieb:
>> The Wyona commons lib contains some new XML helper/utility methods
> 
> A few comments on
> https://svn.wyona.com/repos/public/commons/trunk/src/java/org/wyona/commons/xml/XMLHelper.java
> follows:
> 
> Why using arrays in interfaces, and not Iterable (Java 1.5+) or Iterator
> or Collection?
> 
> Please avoid implementation-specific classes as parameters: HashMap ->
> Map: what if wanted to give the method a sorted map?
> 
> Can someone explain to me what the use case for isFragment is? ...or
> even write a unit test! :)

isFragment is simply to indicate if the XML declaration should be included.

>>         if( document == null ){
>>             log.warn("document to string called with a null document!,
>> don't do this.");
>>             return null;
>>         }

Fair enough, I personally prefer to only throw exceptions when really
necessary. This should be handled by done by the caller.

> I would rather throw a NPE right here with the same comment than getting
> it in a maybe distance place!
> BTW shouldn't we always rethrow exceptions as runtime ones in helper
> classes? Do we otherwise have an exception handling policy?

I'm open to it but I feel a library is better to be explicit about the
exception that it throws. Just throwing a raw exception is not that helpful.

>> I am currently trying to refactor this, whereas I want to write tests
>> first for the existing code, before actually substituting with the
>> helper methods.

That sound perfectly reasonable. However as it stands, If no namespace
is usedm, a NPE will be thrown.

> It would definitely be better to have tests 1st, if only to see how the
> APIs work out/feel in practice in the test cases' code, esp. as regards
> exception handling.

The existing unit tests for the XMLHelper class no longer compile!

Also I notice that this library has more dependencies before.

Another point relates to code sytle, I personally find code like this
awkward to follow when its shown on a single line.

javax.xml.transform.TransformerFactory.newInstance().newTransformer().transform(new
javax.xml.transform.dom.DOMSource(doc), new
javax.xml.transform.stream.StreamResult(out) );


Alec,