Review: Needs Fixing - The implementation looks very clean. Only a couple of minor comments that might help removing some code but I couldn't find any major rejection points. ;-) - ChangeLog entry is missing - I understand why you wanted to rename the function from replace to edit and your concern makes sense. However, the description of the function still uses the term "replace" and I kind of like the explanation as it is. - The comment should mention that you can't replace a node with an object or vice versa (mentioning inconsistent kind with the ZDDY0040 error is not enough for the user). Also, that it's only possible to replace root nodes. - Is there no more efficient way to find out whether a JSONItem is the root of a tree or not? Currently, it seems that the searching only works for ordered collections because it's using findNode. - Could the Item::swap function be implemented on JSONObjects, JSONArrays, and the base-class for all nodes instead of for Item. It seems like that checking if a node is the root and swapping the tree in XmlNode::swap would be sufficient. That seems to be more consistent with the way other functions are implemented. It also seems to avoid the switch in Item::swap and the replication of raising ZSTR0050. -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba.
-- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
