Forgotten patch: qofid.diff
Neil Williams
linux at codehelp.co.uk
Wed Oct 26 17:41:18 EDT 2005
On Wednesday 26 October 2005 8:48 pm, Chris Shoemaker wrote:
> On Wed, Oct 26, 2005 at 07:14:08PM +0100, Neil Williams wrote:
> > On Tuesday 25 October 2005 3:18 pm, Chris Shoemaker wrote:
> > > 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.
>
> It's certainly not strictly necessary, since I could always use
> qof_collection_foreach to generate a GList.
BTW: qof_collection_count already exists.
If we can avoid iterating over a hash_table to append to a GList which we then
iterate over again, I'd prefer that! (Especially as ALL the entities involved
are exactly the same pointers!)
Wherever we put this list function, it's going to mean repeated iteration over
the same entities.
Do you generate this list just once (on book load) or do you keep it in sync
with changing entities in the book? What about the collection?
Can you use the QofCollection for your object ID directly? Let that be your
GList and do all the iteration with qof_collection_foreach instead of using a
second structure in memory?
I can't help with specifics at the moment, I can't see your code (but that
reflects another, different, story).
Do you create budgets as QofEntities with qof_class_register and
qof_object_register? If you use qof_instance_init, the book already contains
a QofCollection and you wouldn't need to store a GList anywhere.
> I just noticed that other
> objects were jumping through various hoops to obtain a GList like
> this. (For example, when they want to get a count of how many such
> objects there are,
use qof_collection_count.
It's relatively new, so older objects haven't been changed to use it yet but
new code should use it.
> or when writing them to file.) I also use it to
> populate a temporary ListStore for the Budget Selection Dialog.
The file example is a little unclear. Certainly in QSF I use
qof_object_foreach_type and qof_object_foreach - that's the basis of the
generic structure. If you know the object type in advance (which you would
have to in order to get the collection) maybe qof_object_foreach would be
suitable? I don't see the example you mention about writing to a file in
gnc-backend-file.
> In every place I used qof_collection_get_list, I could have just
> written a static add_to_list callback, and used qof_collection_foreach
> to get a GList, but I'm lazy and I didn't want to redo this every
> place I needed it, nor even once in Budget's API.
I understand that. I'm just concerned that we would be iterating over the same
entities again and again in two different forms. It seems wasteful to me.
qof_collection_get_list may look convenient but I'm concerned that it's doing
more than it needs behind the simple call.
Can you adapt the functions that call qof_collection_get_list to call a
different callback using qof_collection_foreach - but without building a
GList in between?
Presumably, you get the GList then use an internal for loop or an external
g_list_foreach routine?
The external g_list_foreach callback is the easiest one to adapt. Change the
gpointer value to QofEntity *ent and remove the cast from gpointer to
QofEntity.
e.g. using a g_list_foreach and callback:
my_func_cb (gpointer value, gpointer data)
{
// internal func using a context for other data, cast from data
}
my_func()
{
QofCollection *col;
GList *list;
list = qof_collection_get_list(col); /**< duplicate the hash_table as a
list without changing any content */
g_list_foreach(list, my_func_cb, context);
g_list_free(list);
}
Or if you have internal for or while loops:
my_func ()
{
QofCollection *col;
GList *list;
list = qof_collection_get_list(col); /**< duplicate the hash_table as a
list without changing any content */
while (list)
{
// internal func
list = g_list_next(list);
}
g_list_free(list);
}
The QofCollection version could be:
my_func_cb (QofEntity *ent, gpointer data)
{
// internal func using a context for other data, cast from data
}
my_func()
{
QofCollection *col;
qof_collection_foreach(col, my_func_cb, context);
}
> Since other objects
> were doing similar things, or worse (like managing a GList stored in
> the collection's generic data field) I thought it might be convenient
> for everybody.
Maybe. I think we can exclude SX-book from the examples of similar problems as
the file itself declares it does not use entities in an efficient manner.
qof_collection_foreach uses the same kind of callback that you would use for
g_list_foreach - you just don't have to cast the gpointer back to
(QofEntity*).
I know it's handy to iterate using lists using a simple for loop but I'd
rather not have the QOF API encourage others to use double iteration over
other objects that may have thousands of entities per book.
Just imagine if qof_collection_get_list was called for GNC_ID_TRANS each time
a new one was added or modified.
> Also, I didn't want to rely on known relationships letting me get a
> GList of Budgets from some other object (like the book) as some
> objects do. And, I certainly didn't want to store a GList in the
> generic book data ala commodity_table.
I can understand that too. However, a QofCollection of budgets is always being
maintained on your behalf, within the book.
I realise this is getting quite low level in your budget structure but I would
prefer not to have QOF providing a standard API that actually provides
duplicate iteration.
If you can use qof_instance_init, you have the collection created, updated and
maintained for you. I'm not sure you'd need to store anything in the book
data, it's already in the book as a QofCollection. (And, by definition, you
can write your budgets out in QSF).
In order to call qof_collection_get_list, you must already have such a
collection. I don't see why you want to iterate over that to append to a list
that you then iterate over again.
> All said, it's just a convenience function. If you don't like it, we
> can just put it in the Budget API.
For now, that may be preferable but, IMHO, it would be better to use the
QofCollection directly and not create a duplicate structure.
> (Although there /is/ a weird
> naming issue:
If you need to use GList for budgets, maybe use this routine locally
temporarily but I think it should use QofCollection instead.
I can't see that qof_collection_get_list can be part of QOF.
> other objects have a GList *
> gnc_book_get_{thisobject}(QofBook *book) or something stranger.
I can find only one - SX-book. We already know that isn't using entities
properly, I don't think it's a good model for other objects.
Part of libqof1 and cashutil is to encourage better object design, improving
the API to deprecate methods that cause trouble later on.
> That
> just seems awkward and not too difficult to avoid if you have
> something like qof_collection_get_list.)
The basic problems, to me, are:
1. It's adding iteration on top of other iteration. We take a
g_hash_table_foreach to build a list using a callback and then iterate over
that list. If this was a QofCollection for GNC_ID_TRANS, it could take a
while! That's the risk with making it part of QOF - it gets used out of
context on objects that have nothing to do with budgets.
2. I'm not that happy about passing the GList back to the caller - there's
enough confusion about KVP and GLists because a GList cannot be relied upon
to identify or maintain any particular conventions about entries in the list.
3. I don't follow some of your examples.
> > > +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?
> >
> Well, you can, but that's not what it's for. It's just another type
> of iteration.
I disagree, it's another *layer* of iteration on top of an existing layer.
It's not an alternative iteration, it's not equivalent - it is supplementary
and dependent on the first iteration without modifying any of the pointers
contained within either iteration.
> And sometimes, you *do* need a list.
Sorry, I can't see when that is necessary. Any examples from your own code?
It's hard to see the merit when I cannot appreciate your reasons for your
initial examples.
> It's just a
> question of who should make it.
Maybe. Or maybe it's a question of whether there is a method that only
involves a single layer of iteration.
--
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/05f16027/attachment-0001.bin
More information about the gnucash-patches
mailing list