Re: [PATCH 2/6] core: changed type of date keys to GDateTime

[Guillaume Emont <guijemont igalia com>]

On 30/06/2011 12:23, Juan A. Suarez Romero wrote:
> On Thu, 2011-06-30 at 12:16 +0200, Guillaume Emont wrote:
>>> My point is that looking how other libraries do, including GLib,
>> almost
>>> I never saw the use of a 'const' in other place than strings. Taking
>> a
>>> look, for instance, in the GList, GObject or GTimeVal API, there is
>> no
>>> trail of 'const' in any parameter than strings, even when the
>>> object/struct is not modified.
>> Yeah, and I personally hate them for that, for all the times when I
>> had
>> to do:
>> GList *list_I_can_modify =
>>   g_list_copy((GList *)const_list_I_cant_modify); 
> 
> That's my point: they assume that your 'const_list_I_cant_modify' is a
> 'GList', not a 'const GList'. No function returns a 'const GList *' so
> there is no reason to define it as const.
> 
> If your API has functions handling 'const GList *', then you need to do
> the cast. That's another reason why I would prefer that all our API use
> 'GList *' and not 'const GList *'.
For me, having the parameter a const is not an issue, since it allows
for both const and non-const parameters anyway.
A relevant question though is whether we want grl_media_get_*_date() to
return a const. I see these possibilities:
 - Return a const, which is the value stored in the GrlMedia, like in my
patch
 - Return a non-const, which is the value stored in the GrlMedia. That
means that you implicitely allow outside modification of the GDateTime
stored in the GrlMedia (fortunately, GDateTime is an opaque structure
with no setter, so this can't really happen). In that case, we should
wonder whether to increase the refcount, which would sound logical, but
is inconsistent with what we do with the gchar * stuff.

In the general case, we could also return a const which is a copy of the
original GDateTime. But here, there is no copy function for GDateTime.

All in all, I start to think that we should return a non-const in
_get_*_date(), so that it can be used everywhere, since it does not
bring any risk since the type is unmutable. I still want to use const in
the setter as a hint that we won't modify the value (the type is
unmutable) and to be less restrictive.

Guij
> 
> 	J.A.
> 
> 
> _______________________________________________
> grilo-list mailing list
> grilo-list gnome org
> http://mail.gnome.org/mailman/listinfo/grilo-list
>