Re: [Multi-valued V2 (grilo) 07/12] core: Create property with an initial set of keys

On Tue, 1 Mar 2011 10:50:26 +0100, "Juan A. Suarez Romero" <jasuarez igalia com> wrote:
Signed-off-by: Juan A. Suarez Romero <jasuarez igalia com>
 src/data/grl-property.c |   23 +++++++++++++++++++++++
 src/data/grl-property.h |    2 ++
 2 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/src/data/grl-property.c b/src/data/grl-property.c
index f3d21bd..7605697 100644
--- a/src/data/grl-property.c
+++ b/src/data/grl-property.c
@@ -59,3 +59,26 @@ grl_property_new (void)

   return grlprop;

I miss some gtk-doc in this class explaining in more detail its purpose and when it should be used.

+ * grl_property_new_with_keys:
+ * @related_keys: (element-type Grl.KeyID): list of keys.
+ *
+ * Creates a new place to store related keys and values.

Creates a new property instance that can be used to store related keys and values.

+ * Initially it will contain the specified set of keys with no values.

This constructors looks like something that is half done. I think we should provide users with a constructor with no arguments that just creates an empty property and then we let users set independent key/value pairs on it or we provide them with a constructor that can set both the keys and the values in one go. But this version does not look right to me (and yes, I know you can just set the keys now and set the values later, but still).

+ * Returns: a new #GrlProperty
+ **/
+GrlProperty *
+grl_property_new_with_keys (const GList *keys)
+  GrlProperty *prop = grl_property_new ();
+  while (keys) {
+    grl_data_add (GRL_DATA (prop), keys->data);

Do we have API in GrlData for adding a key with no value like this? Please let's not do strange things. grl_data_add should receive a key and a value.

+    keys = g_list_next (keys);
+  }
+  return prop;

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]