Re: [BRANCHES] Giving some sanity to dates

[Guillaume Emont <guijemont igalia com>]

Hi Juan, and thanks for your comments.

See my answers below, and I will soon update the branch accordingly.

Also, I'd like to ask: was this the fruit of a full review, or only some
preliminary comments with more to come later?

Guij

On 23/09/2011 18:59, Juan A. Suarez Romero wrote:
> Couple of comments:
> 
> In grl-media.c, grl_media_get_publication_date():
> 
> **
>  * grl_media_get_publication_date:
>  * @media: the media object
>  *
>  * Returns: (transfer full): the media's date (TBD)
>  *
>  * Since: 0.1.4
>  */
> 
> 
> It should tell
> 
> * Returns: (transfer full): the publication date of @media
> 
Good catch. Forgot to update that.
> 
> 
> In grl-media.c, grl_media_get_publication_date():
> 
> GDateTime *
> grl_media_get_publication_date (GrlMedia *media)
> {
>   GDateTime *date;
>   date = grl_data_get_boxed (GRL_DATA (media),
>                              GRL_METADATA_KEY_PUBLICATION_DATE);
>   if (date)
>     return g_date_time_ref (date);
>   return NULL;
> }
> 
> 
> Is there any reason to return date as reffed? The same happen in
> grl_media_get_modification_date().

Hmm, it's been a while ago. I probably made it like that out of fear
that _set_modification_date() gets called while we are using the value
returned by the getter without taking a reference.
But arguably, there's the same issue with all the getters of GrlMedia,
and if we document that the returned value remains owned by the
GrlMedia, people should expect to have to handle that properly. I'm OK
to change that (seems to work with bindings too) for consistency.
> 
> My opinion is that we shouldn't return it reffed, and let users to do a
> ref() if they want to keep it.
> 
> The rationale is that all grl_media_get_foo() functions are always
> returning constant information that is owned by GrlMedia. And in all
> cases, specially when you handle strings, you know that you need to make
> a copy of data if you want to keep it.
> 
> So in order to keep it coherent, and not introduce doubts in usesr when
> dealing with the returned data ("must I free the returned value?", "do i
> need to ref it or not?", ...) , my proposal is to return the GDateTime
> without reffing it, and writing (of course) in the doc that the value is
> owned by GrlMedia and user should ref() it if they want to keep it.
> 
> 	J.A.
> 
> 
> _______________________________________________
> grilo-list mailing list
> grilo-list gnome org
> http://mail.gnome.org/mailman/listinfo/grilo-list
>