[Code-review] review: Gadget act-props branch

Guillaume Desmottes guillaume.desmottes at collabora.co.uk
Mon Aug 11 15:21:01 EDT 2008


Le lundi 11 août 2008 à 14:07 +0100, Dafydd Harries a écrit :
> act-props:
> 
> >+        properties = activity_elem.addElement((ns.ACTIVITY_PROP, 'properties'))
> >+        for node in properties_to_xml(activity.properties):
> >+            properties.addChild(node)
> 
> We seem to do this a lot. Let's change it so properties_to_xml returns the
> <properties> element, then we can just say:
> 
>   activity_elem.addChild(properties_to_xml(activity.properties))


The old code used to do that and you asked me to change to this current
form :)


> >+    for child in node.elements():
> >+        if child.uri == ns.ACTIVITY_PROP and child.name == 'properties':
> >+            for child in child.elements():
> >+                assert child.name == 'property'
> 
> I think it's better to just ignore unexpected elements:
> 
>   for child in child.elements():
>       if child.name != 'property':
>           continue
> 
> We could have a convenience function:
> 
>   def elem_find_children(elem, uri, name):
>       for child in elem.elements():
>           if (child.uri, child.name) == (uri, name):
>               yield child
> 
>       ...
> 
> So we could then write:
> 
>   for child in elem_find_chidlren(ns.ACTIVITY_PROP, 'property'):
>       ...
> 
> This also handles namespaces more correctly.
> 

Nice idea. I added this function to util.py but modified it a bit to
handle None as uri (as in this example <property> inherit its NS but
child.uri doesn't contain it).

> Otherwise, looks fine.


I pushed my changes, let me know if you're ok with them.


	G.




More information about the Code-review mailing list