Re: [PATCH 1/9] core: Create relations between keys




On Mon, 21 Feb 2011 10:36:27 +0100, "Juan A. Suarez Romero" <jasuarez igalia com> wrote:
A couple of keys are related when getting a value for one of the keys, it is
expected that the other key has also a value.

It is intended to be used with keys providing multivalued data: for
each single
value in one key, there must be a value matching that single value. This matched value can be NULL, meaning that it doesn't have actually a value, but at least it is possible to create a relation between values in both keys.

Signed-off-by: Juan A. Suarez Romero <jasuarez igalia com>
---
 src/grl-plugin-registry.c |   89
+++++++++++++++++++++++++++++++++++++++++++++
 src/grl-plugin-registry.h |    7 ++++
 2 files changed, 96 insertions(+), 0 deletions(-)

diff --git a/src/grl-plugin-registry.c b/src/grl-plugin-registry.c
index 20ee3a8..c5944c5 100644
--- a/src/grl-plugin-registry.c
+++ b/src/grl-plugin-registry.c
@@ -61,6 +61,7 @@ struct _GrlPluginRegistryPrivate {
   GHashTable *configs;
   GHashTable *plugins;
   GHashTable *sources;
+  GHashTable *relation_keys;

I would rename this to 'related_keys'.

   GParamSpecPool *system_keys;
   GHashTable *ranks;
   GSList *plugins_dir;
@@ -130,6 +131,8 @@ grl_plugin_registry_init (GrlPluginRegistry *registry)
     g_hash_table_new_full (g_str_hash, g_str_equal, NULL, NULL);
   registry->priv->sources =
     g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
+  registry->priv->relation_keys =
+ g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, NULL);
   registry->priv->system_keys =
     g_param_spec_pool_new (FALSE);

@@ -798,11 +801,76 @@ grl_plugin_registry_register_metadata_key
(GrlPluginRegistry *registry,
     g_param_spec_pool_insert (registry->priv->system_keys,
                               key,
                               GRL_TYPE_MEDIA);
+    /* Each key is related with himself */
                                   ^^^^^^^ itself :)

+    g_hash_table_insert (registry->priv->relation_keys,
+                         key,
+                         g_list_prepend (NULL, key));
     return key;
   }
 }

 /**
+ * grl_plugin_registry_register_metadata_relation:
+ * @registry: the plugin registry
+ * @key1: key involved in relationship
+ * @key2: key involved in relationship
+ *
+ * Creates a relation between @key1 and @key2.
+ *
+ * It make sense when using multiple values: means that for each
         ^^^^^^^^^^
         makes sense
value in @key1
+ * there must be also a value in @key2 (which can be NULL).
+ **/
+void
+grl_plugin_registry_register_metadata_relation (GrlPluginRegistry *registry,
+                                                GrlKeyID key1,
+                                                GrlKeyID key2)
+{

I would rename this API to register_metadata_key_relation. That is, I would add _key_. The same for the rest of this API set.

+  GList *key1_partners, *key1_peer;
+  GList *key2_partners, *key2_peer;
+
+  g_return_if_fail (GRL_IS_PLUGIN_REGISTRY (registry));
+  g_return_if_fail (key1);
+  g_return_if_fail (key2);
+
+  if (key1 == key2) {
+    return;
+  }
+
+  /* Search for keys related with each key */
+ key1_partners = g_hash_table_lookup (registry->priv->relation_keys, key1); + key2_partners = g_hash_table_lookup (registry->priv->relation_keys, key2);
+
+  /* Check if they are already related */
+ if (!key1_partners || !key2_partners || key1_partners == key2_partners) {
+    return;
+  }


Why do you exit if key1_partners *or* key2_partners are NULL?
Also, can that even happen given that you relate each key with itself?

+  /* Merge both relations */
+  for (key1_peer = key1_partners;
+       key1_peer;
+       key1_peer = g_list_next (key1_peer)) {
+    /* Search key1 peer in key2 partners */
+    for (key2_peer = key2_partners;
+         key2_peer && key2_peer != key1_peer;
+         key2_peer = g_list_next (key2_peer));
+    if (!key2_peer) {
+      /* Didn't found; add it to key2 partners */
+      key2_partners = g_list_prepend (key2_partners,
+                                      key1_peer->data);
+    }
+  }

In the inner loop you check key2_peer != key1_peer. Is this even possible? I mean if there is a key in key1_partners that is already in key2_partners shouldn't key1_partners and key2_partners be the same (and thus never reach these loops)?

Iago


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