GNOME.org
 

Re: Removing empty keys from GrlData
On Tue, 29 Mar 2011 14:07:39 +0200, "Juan A." Suárez Romero <jasuarez igalia com> wrote: 
On Fri, 2011-03-25 at 18:39 +0100, Juan A. Suárez Romero wrote:

So I'm going to remove that possibility from GrlData. That is, all
keys
in GrlData will need to have a value.
While working on this, I found some 'doubts' about a couple of functions 
in GrlData.

As reminder, GrlData handles keys (and related keys) that can have
multiple values.
As example, let's think in url, and two of its related keys, mime-type, 
and bitrate.

Let's figure out that our media has 5 urls, but only 3 of them have a
mime-type. And none of them have a known bitrate. As example

{ "http://world.com/1.oth";, NULL, NULL }
{ "http://world.com/2.mp3";, "audio/mp3", NULL }
{ "http://world.com/3.unk";, NULL, NULL }
{ "http://world.com/4.ogg";, "audio/ogg", NULL }
{ "http://world.com/5.wav";, "audio/wav", NULL }


So we have added those 2 related keys in our GrlData. Now the doubts
1.a) What grl_data_has_key (data, GRL_METADATA_KEY_URL) should return? 1.b) What grl_data_has_key (data, GRL_METADATA_KEY_MIME) should return? 
1.c) What grl_data_has_key (data, GRL_METADATA_KEY_BITRATE) should
return?
In current implementation, all functions return TRUE. The reason is that 
this function returns TRUE if the asked key or any related key has a
value.
I'd say that if we are forcing that all the keys must have a value, then has_key cannot return TRUE for BITRATE, because it does not actually have a value. 



2) What grl_data_get_keys (data) should return?

In current implementation, it returns the three keys:
grl_data_get_keys() returns all keys in GrlData as well as the related 
keys.
I think this has something to do with the work to do here: seems logical 
that bitrate shouldn't be returned, as it does not have any value.

But what about mime? If get_keys() only checks the first value, then
clearly it should only return url. But if it checks all values, then
mime should also be returned.

Thus, we should decide if this function should only check the first
value or not.
I think we should return the keys for which there is at least one known value. 
In the example I think it should return url and mime, but not bitrate.
This must be consistent with the behaviour of has_key(). If has_key(k) returns TRUE get_keys() must return 'k' too. 


3.a) What grl_data_key_is_known (data, GRL_METADATA_KEY_URL) should
return?
3.b) What grl_data_key_is_known (data, GRL_METADATA_KEY_MIME) should
return?
3.a) What grl_data_key_is_known (data, GRL_METADATA_KEY_BITRATE) should 
return?

In current implementation, this function only checks the first value,
and thus would return TRUE, FALSE, FALSE.
I think this "keys is known" made sense when we allowed keys that did not have a value. I think it does not make sense now, or am I missing something? 



4.a) What grl_data_length (data, GRL_METADATA_KEY_URL) should return?
4.b) What grl_data_length (data, GRL_METADATA_KEY_MIME) should return? 
4.c) What grl_data_length (data, GRL_METADATA_KEY_BITRATE) should
return?

In current implementation, all of them returns 5: checks how many
elements the key and related keys have.

And this has something to do also with this proposal: shouldn't this
function return 5, 3 and 0 respectively?
We should ask ourselves why would someone use length() to begin with. I can see two reasons:
1) Because they want to traverse the list of values: in that case all the functions should return 5. 2) Because they want to know how many media resources are available: in that case all the functions should return 5.
I don't see a clear case where the user would be interested in knowing the number of all non-null values, but if there is we should probably add specific API for that and keep the current one as is (or maybe add a gboolean parameter to the current API to control whether we are interested in NULL values or not). 



5.a) What grl_data_get_all_single_related_keys (data,
GRL_METADATA_KEY_URL) should return?
5.b) What grl_data_get_all_single_related_keys (data,
GRL_METADATA_KEY_MIME) should return?
5.c) What grl_data_get_all_single_related_keys (data,
GRL_METADATA_KEY_BITRATE) should return?


In current implementation (which btw doesn't honour to what
documentation says), all of them returns the list with the 5 related
keys with their values.

Now, I wonder if (5.a) shouldn't return a list with the 5 url values,
(5.b) a list with the 3 mimetypes values, and (5.c) an empty list.
Well, wasn't this intended to just get all values for a key without caring about related keys at all? In that case I guess it should do just that. That is, it should do what you suggest and not what is doing at the moment.
BTW, I don't like the name we chose for this at all, its behaviour is not obvious from its name: - Why the heck did we name this with the words "related_keys" in there if this is supposed to ignore related keys all together? - Why is this named _get_all_single_related_keys if this is supposed to return values and not keys?
I suggest we rename this to something more descriptive like: grl_data_get_single_values_for_key() or something along this line. 

Iago







Thanks in advance for your opinions.


	J.A.


_______________________________________________
grilo-list mailing list
grilo-list gnome org
http://mail.gnome.org/mailman/listinfo/grilo-list
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]