[sugar] review: gadget buddy search code

[Guillaume Desmottes]

Le jeudi 15 mai 2008 ? 20:33 +0100, Dafydd Harries a ?crit :
> Ar 15/05/2008 am 15:04, ysgrifennodd Guillaume Desmottes:
> > Le mercredi 14 mai 2008 ? 22:52 +0100, Dafydd Harries a ?crit :
> > > > +# FIXME: add more types
> > > > +type_to_str = {str: 'str'}
> > > > +str_to_type = {'str': str}
> > > > +
> > > 
> > > We should check which types the buddy/activity interfaces support for
> > > properties, and test each of those types.
> > > 
> > 
> > Gabble supports the following types: str, bytes, int, uint and bool.
> > 
> > I started to write tests for parse_properties and properties_to_xml but
> > I encountered a problem.
> > Currently we store properties in a dict like {'key': (type, 'value')} as
> > {'color': (str, 'red')} for example.
> > 
> > But how should I store an uint as Python doesn't have an uint type?
> > Using dbus type? Or we could store the string representation of the type
> > instead of a python type ('str' instead of str).
> > 
> > And how should we store the value? Using the type-converted value (so
> > the numerical value for int/uint, True/False for bool, etc) or the
> > string version?
> 
> The only thing the value representation matters for is searching. Storing them
> as strings should be ok as long as there is only one string representation for
> each value.
> 
> I'm not sure that the representation of the type matters. We should do
> whatever is simplest. I suspect that might be storing the types as strings,
> but I'm not sure.
> 

As you said, the only purpose of these properties is for searching so
storing both values and types as string is the easiest solution.

I changed that and added support to uint, bool and bytes types.


> > +            self.log("Ignore activity properties message from %s containing an invalid property"
> 
> This should be in the present tense ("Ignoring...") or the past tense
> ("Ignored..."). I think I prefer the former.
> 

fixed.

> Sure. But:
> 
> +class BuddyIqTest5(TestCase):
> +    """buddy multi query with two results"""
> 
> +class BuddyIqTest5(TestCase):
> +    """buddy query with an invalid property"""
> 
> Let's rename the second one to BuddyIqInvalidPropertyTest or something like
> that.
> 

done.

> Otherwise, go ahead and merge.
> 

merged, thanks for your comments


	G.