Forgotten patch: qofid.diff

[Neil Williams]

On Tuesday 25 October 2005 3:18 pm, Chris Shoemaker wrote:
> Subject: [qofid.diff] Minor tweaks to Qof Code in src/engine.
>
> Neil, I don't know if this patch messes up your plans.

No, but I am curious about why some of it is needed.

Paid employment commitments prevented me replying earlier.

> The only 
> essential thing here is qof_collection_get_list().  If we have to, I
> suppose we could pull this into the budget namespace until the next
> qof release.  What do you think?

I'm not sure about the GList - I'd like to know how you want to use it and why 
qof_collection_foreach isn't adequate.

>    - Introduced qof_collection_get_list() for getting GList of entity
> GUIDs.

It's QofEntity that you are actually storing in the list rather than the GUID. 
In which case, I'm not sure I see the advantage over qof_collection_foreach.

> @@ -158,8 +158,8 @@ void
>  qof_collection_destroy (QofCollection *col)
>  {
>    CACHE_REMOVE (col->e_type);
> -  g_hash_table_destroy(col->hash_of_entities);
>    col->e_type = NULL;
> +  g_hash_table_destroy(col->hash_of_entities);

(separate minor point):
Why move the g_hash_table_destroy call?

> +static void
> +add_to_list(gpointer key, gpointer val, gpointer data) {
> +    GList **list = data;
> +    *list = g_list_append(*list, QOF_ENTITY(val));
> +}
> +
> +GList *
> +qof_collection_get_list(QofCollection *col) {
> +    GList *list = NULL;
> +    g_return_val_if_fail (col, NULL);
> +
> +    g_hash_table_foreach(col->hash_of_entities, add_to_list, &list);
> +    return list;
> +}

Why do you need this GList call at all? I'm not sure I see why you need to 
convert the QofCollection hashtable into a list. What does it gain compared 
to qof_collection_foreach?

> +/** Get a list of entities in the collection.  Caller owns the list. */
> +GList * qof_collection_get_list(QofCollection *col);

To me, this defeats the main purposes of a primary collection - to keep the 
e_type identifier and the GUID hash table key. You can't look up entities 
from the converted list and the list isn't limited to only one e_type.

-- 

Neil Williams
=============
http://www.data-freedom.org/
http://www.nosoftwarepatents.com/
http://www.linux.codehelp.co.uk/

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.gnucash.org/pipermail/gnucash-patches/attachments/20051026/04eaeed7/attachment.bin