|Subject:||Re: NSXML* classes|
|Date:||Thu, 23 Feb 2012 09:18:10 -0700|
Thanks for reviewing this code!
On Feb 23, 2012, at 2:37 AM, Fred Kiefer wrote:
First, let me assure you that we have code in our project that does exercise the current retain count mechanism and appears to be working properly. I agree that it would be good to add some GNUstep tests to work with this as well. One type of scenario that should be tested would be to create an NSXMLDocument from an XML string. This will create only the top-level object. Then access a node farther down the tree, either by calling childAtIndex: a couple of times to get a grandchild, or alternatively by using an XPath call to go directly to some descendant node (or better yet, test both scenarios). The code will automatically create Objective-C objects for the nodes that are accessed as well as each node in the parent chain leading from that node up to the document.
After obtaining a lower-level node object, release the document object. None of the ObjC objects that were created should be dealloc'ed as long as there is a retain on any of them from somewhere outside of the tree. Upon releasing the lower-level node, all of the objects in the tree should be dealloc'ed and the underlying libxml2 tree structure will be freed. To test this thoroughly may require wrapping the test in an autorelease pool.
I agree that isEqual: should not check the sibling nodes. We'll get that fixed. The test needs to be performed on the libxml2 structure, though, not at the ObjC level. The whole philosophy of the current implementation is that essentially all of the data resides in the libxml2 tree. ObjC objects are only created as needed, for nodes that are being accessed (plus their direct ancestors, as noted above for the memory management system). Creating ObjC objects for every node in two trees in order to test whether they are equal would be wasteful and entirely counter to this philosophy.
I haven't really worked with namespaces, so I don't currently have an opinion on this although it sounds like you have made some valid points. I'll try to take a look at it later, after we wrap up our current release.
|[Prev in Thread]||Current Thread||[Next in Thread]|