kvp_value_get_glist - Chris' confusion.

[Chris Shoemaker]

On Sat, Feb 19, 2005 at 04:04:46PM +0000, Neil Williams wrote:
> Chris, about your comment in kvp_frame.h:
> 
> >  CAS: Maybe I'm confused (because this case really is too
> >  confusing), but I think this comment is wrong.  I think the
> >  returned GList is a list of whatever values were added by
> >  kvp_frame_add_*().  So, if you called kvp_frame_add_frame, then,
> >  yes, it's a list frames, but if you called
> >  kvp_frame_add_{something_else}(), then it's a list of
> >  something_elses.
> > 
> >  IMO, this glist case SHOULD NOT BE USED.  Why would you make a
> >  KvpValue a glist of frames when a frame is already inherently a
> >  list of subframes?  And why would you make a KvpValue a glist of
> >  any other type when, apparently, the interface that requires you to
> >  specify value types doesn't enforce type consistency in the list,
> >  and provides no record of the type anyway?  Talk about asking for
> >  headaches.  (I'm too chicken to search for callers. :)
> > 

Somebody reads my drivel?!  :)

> 
> I think I can help solve your confusion.
> 
> 1. Use my doxygen output if you ever want to know about the callers - I've 
> configured doxygen to use the references (which Derek doesn't like to use on 
> the GnuCash server) and it makes it easy:
> http://code.neil.williamsleesmill.me.uk/doxygen/group__KVP.html#ga78
> That lists the callers (and the callees).

I've looked at this a bit, but I'll look some more.  Thanks for hosting this.

> 
> 2. The key to kvp_value_get_glist is kvp_value_glist_to_string. That's on
> line 01613 of kvp_frame.c:
> tmp3 = kvp_value_to_string((KvpValue *)cursor->data);
> 
> So the GList contents must be KvpValues - any KvpValueType - not just frames.

That makes more sense than a glist of frames - a glist of kvp_values.

> 
> The list allows you to have a list of values (any known type) on a single key. 
> (It's also recursive in that you could have a list of lists, but I don't 
> think anyone actually does that!)
> 
> So, the comment should read:
> 
>  The returned GList is a list of  KvpValues of any type, including mixed. 
>  Cast each list gpointer to a KvpValue and use the KvpValue->type to
>  determine the type of value at each position in the list.

Much clearer.

> 
> I'll gladly change it to that as I need to make a commit in kvp_frame anyway 
> this weekend.
> 
> To answer your specific queries in the comment:
>  IMO, this glist case SHOULD NOT BE USED.  Why would you make a
>  KvpValue a glist of frames when a frame is already inherently a
>  list of subframes?
> 
> You don't, you hook a list of values to a single key, if that is necessary. 

Yeah, that's why it didn't make sense.

> I'm in the middle of coding how this is to be represented in QSF, lists and 
> binaries are the only remaining bits to sort out.
> 
>   And why would you make a KvpValue a glist of
>  any other type when, apparently, the interface that requires you to
>  specify value types doesn't enforce type consistency in the list,
> 
> You don't need consistency beyond all are KvpValues.

IOW, type-consistency _is_ enforced: all elements are KvpValues.

> 
>  and provides no record of the type anyway?
> 
> Any KvpValue contains it's own type! Just cast the list gpointer to a KvpValue 
> and the type is immediately available.

I get it now.

> 
>   Talk about asking for
>  headaches.  (I'm too chicken to search for callers. :)
> 
> Then use doxygen to do the search for you.
> 
> Does that explain things?

Very well.  Thanks for explaining that.

-chris