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

[Guillaume Emont <guijemont igalia com>]

On 30/06/2011 13:36, Guillaume Emont wrote:
> 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.
That is in an ideal world. But right now, the introspection stuff has
issues with GDateTime (does not recognise it is a GBoxed), and is only
happy if we return the value as a constant. So I will leave the const
return, as a workaround for that.

Guij