Re: [Multi-valued V2 (grilo) 11/12] core: Merge GrlDataMulti into GrlData

[Iago Toral <itoral igalia com>]

On Thu, 03 Mar 2011 09:54:08 +0100, "Juan A." Suárez Romero 
<jasuarez igalia com> wrote:
> On Wed, 2011-03-02 at 16:06 +0000, Iago Toral wrote:
> > > either
> > > you remove all related values, or remove nothing.
> >
> >  Good, I think that is what we should have.
> >  Then I think we should rename grl_data_remove_property to
> >  grl_data_remove.
>
> The point of naming it grl_data_remove_property() instead of
> grl_data_remove() is that there is already a grl_data_remove() 
> function
> in GrlData: removes value for first key.

:)

Well, that is exactly what I asked before and you answered that we did 
not have that API, I guess you did not understand my question. :)

> Note that I've kept all functions that was in GrlData previously, 
> which
> were working with single-valued data (now, internally they work with 
> the
> first value of key).
>
> We can discuss if we can get rid of some of this functions, but that
> would change the API, so I prefer to keep it as it is, at least for 
> some
> time, after we think which functions can be removed, and what 
> functions
> should be renamed.

No, it won't change the API.
What I am saying is that we should have grl_data_remove *only* but its 
implementation should be that of grl_data_remove_property and we should 
not have grl_data_remove_property, of course.

> > > But users can still remove a specific element if they really want:
> > > get
> > > the property with the element, and then remove the key from that
> > > property.
> >
> >  Shouldn't we remove that API from GrlProperty then?
>
> The API to remove the key in GrlProperty is there to implement the
> grl_data_remove(), which acts on single-values (i.e., the first value 
> of
> multi-valued keys).

If it is there for internal use as you suggest here, then that's fine 
with me, but then it should not be part of the public API, since as I 
said, its use by clients could break the semantics of the related keys, 
and I don't really see a good reason why they would like to use this 
instead of grl_data_remove (updated to deal with related keys)

> Of course, I can still get rid of it, as doing
>
> grl_property_remove(prop, key)
>
> is almost the same as
>
> grl_property_set (prop, key, NULL)

But this is not exactly the same, as grl_property_set would actually 
leave the key with NULL value.

Iago