On 05/26/10 03:21 AM, Dermot McCluskey wrote:
Keith,
Comment re: non-recursive to_xml() below.
3.4.1.3
to_xml: What's the reasoning behind not having the object itself make
the recursive call into to_xml()? In particular, the restriction it
creates - "it is possible... for the to_xml() method to generate the
complete tree... In this case the child objects' to_xml() methods must
return None" - seems very unintuitive.
The main reason is to remove the burden of it for the implementors.
But as in my response to Dave Miner, yesterday, it does make sense
to have a
mechanism to prevent further recursion - so that the parent node can
make the
decision to prevent it if desired - but by default it would descend
the tree.
A 'for' loop around its children doesn't seem terribly burdensome; on
the other hand the additional complexity of supporting two options
here seems like a hassle to me.
If there's a need to support two paths, splitting up the functions
would be smoother:
def to_xml():
'''Never overridden'''
my_xml = self.as_xml()
my_xml.append(self.children_as_xml())
return my_xml
def as_xml():
'''Abstract method; subclasses return an XML object representing
itself'''
...
def children_as_xml():
'''Only overridden by a subclass if it wants to take control of
adding its children to the XML'''
children_xml = SomeXMLObject()
for child in self.children:
children_xml.append(child.to_xml()) # Note this is a call
into 'to_xml', not 'as_xml'
return children_xml
It's a bit more complicated. We want to support the ability
for any node in the tree of objects to return None from to_xml(),
even if its parent and children all return some XML. ie the
XML can 'skip a generation' as in this (totally artificial) example:
Cache Object Structure vs XML representation
============================================
[Discovered targets] # <DiscoveredTargets>
-> [Logical targets] # None
-> [Disk 1] # <Disk name="...">
-> [Disk 2] # <Disk name="'''>
...
So, the XML for the disks is not appended to their parent, but to
their grandparent. In your example, you'd end up doing
None.append(<Disk ...>) which would fail.
For obvious reasons I don't generally do error checking / edge cases in
pseudo-code.
Obviously, this can be coded, but it's more complicated that your
example above and we'd rather do this once in the base class
definition rather than force all the sub-classes to implement it.
In my example, to_xml is only written once in the base class (subclasses
overwrite as_xml()), so the check for None doesn't have to be handled by
the children. Re-writing that function slightly results in the intended
behavior:
def to_xml():
'''Never overridden'''
my_xml = self.as_xml()
if my_xml is None:
return self.children_as_xml()
else:
my_xml.append(self.children_as_xml())
return my_xml
On a different note, I'm also a bit confused by the example you gave -
extending it further:
Cache Object Structure vs XML representation
============================================
[Discovered targets] # <DiscoveredTargets>
-> [Logical targets] # None
-> [Disk 1] # <Disk name="...">
-> [Disk 2] # <Disk name="'''>
-> [Virtual Targets] # None
-> [Zpool 1] # <Zpool ...>
The proposed solution sounds like any DataObject that returns None from
to_xml() is risking having all it's child data dumped into a giant
bucket with who knows what. I think it'd be worth calling that out
explicitly in the design doc. My first impression for returning None
from to_xml() would have been that it effectively cuts off the tree at
that point. This comes from the mindset, of course, that the to_xml()
methods are descending into the tree (which seems the more intuitive
approach, from the standpoint of someone implementing a subclass of
DataObject).
Now that I've sidetracked myself a bit, I still am unsure of the benefit
of having 2 paths for XML generation is here. I think the overall design
would be simplest if each node were responsible for itself, and for its
children. The requirement that generates is nothing more than that a
to_xml() function for a class should have a "for" loop that adds
children to itself - and that really doesn't seem that hard: it's simple
in the basic case, and gives the individual node control over what the
XML looks like. Given that the node is supposed to know more about the
XML output than the DOC, this seems to make more sense.
I think that this discussion might be best continued verbally, however.
At least, I've reached the point where I'm having a difficult time
expressing my point (whether it's a valid point or not!)
- Keith
- Dermot
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss