Re: [PATCH 1/5] local-metadata: udpated to may_resolve() API

[Guillaume Emont <gemont igalia com>]

On 24/02/2011 13:55, Juan A. Suárez Romero wrote:
> On Tue, 2011-02-22 at 16:32 +0100, Guillaume Emont wrote:
>>  
>> -  if (key_id == GRL_METADATA_KEY_THUMBNAIL)
>> -    return deps;
>> +  if (missing_keys)
>> +    *missing_keys = grl_metadata_key_list_new (GRL_METADATA_KEY_URL,
>> NULL);
>>   
>
> I think it would be better to use g_list_append() than
> grl_metadata_key_list_new()
>
>
> Thus, we could invoke may_resolve() using the same missing_keys list for
> all keys, collecting all the missing keys in the same list.
>
As I said on IRC, I prefer the semantic "may_resolve() create a list"
than the one where "may_resolve() updates a list".
Just because it's more obvious what you're doing if you do something like:
may_resolve(source, media, key, &tmp)
my_key_list = g_list_concat(my_key_list, tmp)

And in the case of the algorithm in core that uses it, IIRC, it's better
to make a union of lists than to concatenate them, so we need to create
a new one (or have awful semantics).
I might change my mind if someone bring very good arguments, but for
now, I feel that this semantic obeys better to the "least surprising"
idea that an API should convey.
It may make sense to add to the documentation of the may_resolve()
vmethod that *missing_keys should be considered to be undefined.

Guij