[Bug 30423] Provide convenience API for GVariant-based a{sv}s

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=30423

--- Comment #1 from Danielle Madeley <danielle.madeley at collabora.co.uk> 2010-10-11 18:11:40 PDT ---
Review thoughts:

+ * If @dict is a floating reference, ownership will be taken as if via
+ * g_variant_ref_sink(). Otherwise, ownership will not be taken.

This is very confusingly worded. As far as I know, it always holds a reference,
but seems to imply it will not.

+TpVariantMap *
+tp_variant_map_new_frozen (GVariant *dict)

You don't think new_from_variant() is a clearer name? You can explain in the
text that it's frozen. Or new_frozen_from_variant(), though that's getting
long.

Why are TpVariantMaps not reference counted? Like hash tables, they seem like
the sort of thing you may wish to reference count.

There's set() and set_value(), and get_value(), but no get().

+TpVariantMap *
+tp_variant_map_copy (TpVariantMap *self)

This is the only way to unfreeze a variant map, which is potentially a pain in
the neck. It seems like there's also use for an unfreeze() method that works
similarly to this method... but dropping the ref on the variant. If that
exists, then perhaps copy() should just make an exact copy, which would be less
confusing.

I wonder about 86b2b165bfde719fa9c5401b4dda15a530274740 .. I see what you're
doing, but I'm worried about people doing: set_value (set_value (set_value
(set_value (tp_variant_new (), v1), v2), ... ) -- everyone becomes a closet
Lisp programmer. I was going to suggest making tp_variant_map_new() behave like
tp_asv_new(), but I don't like that idea either.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.