On Saturday 28 July 2007 00:26:19 Maciej Stachowiak wrote:
On Jul 27, 2007, at 11:36 AM, Lars Knoll wrote:
On Friday 27 July 2007 16:50:01 Darin Adler wrote:
I'm not happy with the code organization here. XMLTokenizer now has tons of ifdefs and two separate implementations. It's fine to have a QXmlStream implementation, but the two implementations should be in separate files, side by side, as we do in the platform layer.
I agree that the #ifdef's are not optimal, and we can move parts of the code out. But currently a lot of code is still shared between the QXmlStream and the libxml based implementations, so we have two choices:
1. just add an XMLTokenizerQt.cpp and duplicate lots of code. 2. add a qt/XMLTokenizerQt.cpp and a libxml/XMLTokenizerLibXml.cpp and keep a common XMLTokenizer.cpp for code that is used in both. 3. Have some ifdef's in the one file.
The first option is no good, as we'd have to duplicate lot's of shared code leading to maintenance issues in the long term.
The second option is better, but also here we'd have quite a bit more duplicated code than required as long as we don't do some bigger refactoring to abstract out the common parts. This is something we wanted to avoid at the moment. So we went for option 3.
Other organizations have requested the ability to use other XML parsers as well, such as expat. Seems like in the long run we want a different approach than just ifdefs in the XMLTokenizer.cpp file. It seems like the best would be some abstraction layer on top of the parser library, but if that is difficult then your option #2 sounds like a docent long-run approach. I would have expected just about every XML parsing library to have a SAX-like API, which shouldn't be too hard to abstract, but perhaps QXml works differently.
I guess that assumption doesn't hold. QXmlStream is a streaming parser with an API that is very different from SAX. It IMO a whole lot simpler to use than a SAX like API and is inspired from similar APIs in the Java world. If you're interested, have a look at http://doc.trolltech.com/4.3/qxmlstreamreader.html Cheers, Lars