New QOF type, QOF_TYPE_COLLECT and recursive entity copying

Tue May 10 10:18:59 EDT 2005

On Tuesday 10 May 2005 1:18 pm, Derek Atkins wrote:
> > GList* gncInvoiceGetEntries(invoice) already existed - it was already
> > public.
> >
> > I didn't change that. QOF cannot deal with the GList directly so I use a
> > QofCollection instead. Then all I've done is a set routine to complement
> > the
> >
> > get routine that already existed.
>
> Why can't QOF be changed to deal with the GList directly?

It could be a GList of anything - QOF needs to know the type. QOF also needs 
to know that each item on the list is the same type as the last one.

> Why does the 
> "convert the GList into a collection" code have to be duplicated in every
> place where it has a list of objects, rather than putting that code in a
> central place in QOF/QSF?

Something like:
QofCollection* qof_collection_from_glist (QofIdType type, GList *list); ?

That would only save a few lines per object.

	entry_coll = qof_collection_new(GNC_ID_ENTRY);
	for(list = gncInvoiceGetEntries(invoice); list != NULL; list = list->next)
	{
		entry = (QofEntity*)list->data;
		qof_collection_add_entity(entry_coll, entry);
	}

becomes:

	entry_coll = qof_collection_from_glist(GNC_ID_ENTRY, \ 
gncInvoiceGetEntries(invoice));
	return entry_coll;

Is it worth it? Wouldn't the redirection from the business module to the 
engine and back be more detrimental?

The QofCollection *to* GList conversion does need to be in the object - only 
the object understands what is required from the collection. It's up to this 
QofSetterFunc to determine that the supplied type is acceptable (from a list 
of suitable types within the object) and that if only one entity should be 
set that it either takes just one from the collection or refuses the entire 
collection. This handling is highly object-specific.

> > passed back to the invoice as one. The collection is also compared as one
> > - using a deep compare routine. During a merge, the collection is updated
> > as a
> > single block.

I got that wrong in the email text above, sorry to confuse you. The correct 
version is in the original email about QOF_TYPE_COLLECT:

merge would be additive - if the type matches, the target collection would 
have all entities from the input collection that do not already exist in 
target, added.

That is how I've implemented the QofSetterFunc for the collection in 
GncInvoice.

Merge is generally additive, we aren't trying to synchronise QofBook's here or 
remove data.

> When you merge accounts do you 
> merge the whole list of transactions as a single block?

When updating an account in a merge, transactions that do not already exist 
will be added. That's why I like QofCollection - the internal hash table 
makes it easy to check that this will happen. A GList is more work to check 
that it does not already exist in the list.

> > ?? How else am I to obtain and set the entries for the invoice? GncEntry
> > is exported in the current v2 XML so it isn't transparent like Owner. QOF
> > has to be able to create the entries for the invoice and this means
> > accessing the list. Doesn't it?
>
> In the v2 gnc-xml the GncEntry is a top-level object, just like a
> Transaction. It's exported separately (just like transactions) and loaded
> and linked via GUID references.  It's not exported as a subtree.

The only "sub-tree" is the list of references exported as part of the invoice 
object. Each GncEntry will still exist as a genuine object elsewhere in the 
QSF.

> Note that it IS possible for a single GncEntry to be referenced by TWO
> invoices! In particular it can be referenced by a Vendor Invoice and then
> again by a Customer Invoice.

That's fine. I have no problem with that. One GncEntry, two <collect> tags in 
two different objects containing the GUID of that entry. That's OK.

> > The business objects are written to v2 XML via Scheme

> HUH???  There's no scheme at all involved in writing the business objects
> into v2 gnc-xml.

Ooops! Yes, I've found that now. Sorry about that.

> > I don't understand why I cannot use a secondary collection typed list for
> > pointers to the entries within the invoice. OK, I know it's a hash table
> > internally, but that is why it is so useful - it's so easy to lookup and
> > ensure you aren't referencing the same entity twice.
>
> See above about how you handle the fact that the same entry could be in
> multiple invoices.  Also, I don't think that code belongs in the invoice
> module, and it certainly doesn't belong in the public api.

Each QOF_TYPE_COLLECT parameter has it's own QofCollection so there's no 
hassle with two collections referring to the same entity.

When I create qof_ prefix functions that standardise the way objects provide 
and handle their data, would it be preferable to put those into the relevant 
objectP.h file instead of object.h ?

I do need to rationalise the book_merge and QSF API as well, so it's a general 
thing. If you've got any advice on what merits public vs private, I'll work 
on that. Generally, I find private headers to be a royal PITA, the learning 
curve for new developers becomes even steeper. Maybe we should use some more 
Doxygen modules to document those parts that are private but which anyone 
needing to understand the engine, business or GUI core can find.

Many of my problems have come down to undocumented parts: the gnc v2 xml, the 
GUI, the druids. I feel that hiding the structure of Account, QofParam, 
QofBackend and GncInvoice makes the doxygen output less useful generally.

I'm hoping to fold all the .txt files listed here:
http://code.neil.williamsleesmill.me.uk/otherdocs.html#GNUCASHDOCS
into the main Doxygen output in the next month or so.

I'd appreciate your input on why structures like the ones I've listed are 
hidden because I want to encourage the development of new backends and new 
methods by making all such structures easily accessible in the QOF doxygen 
documentation.

More often than not, the source just ain't enough.

A particular problem is the linking within Doxygen. If the struct is hidden or 
undocumented, every appearance of the type is unlinked. It makes it much 
harder to cross-reference different parts of the API if, whilst viewing the 
GUI merge code, you can't just click on QofSession and find the documentation 
for the backend API.

http://code.neil.williamsleesmill.me.uk/gnome2/group__NewHierarchy.html

So if you need to know how to use the GUI merge code, you get no clues about 
how to find out about the sessions that it uses.

> > It's quite separate from qof_book_get_collection - at no point is the
> > entity itself changed, it remains within it's primary collection. Adding
> > an entity to a secondary collection does not change the primary. It's
> > about references and recursion.
>
> Fair enough..  Except that in the case of a GncOwner how do you make sure
> that you have a collection with one and only one entity?  (I've already
> mentioned the multiple-reference thing ;)

It's up to the relevant setter function. If the collection contains the wrong 
type of entities, it must be rejected, naturally. That's an assertion failure 
in most cases as it indicates something has gone badly wrong with the object 
typing. These collections will have come from genuine objects so there should 
be no reason for a wrong type.

Similarly, if the object is supplied with multiple entities when it can only 
cope with one (as in the case of GncOwner), it's hard to see how that can 
arise without a programming error. I can't see a way for a user to create 
such a fault because if too many entities exist in the object, too many 
entities must have been set in another object of the same type in the first 
place! 

QOF relies on the setter functions doing sensible things! If the user botches 
a hand-edit of the XML, attempting to set multiple owners needs to trip an 
error in the set function when the QSF file is loaded and the new invoice is 
first populated. The set function can use an assert failure or even set a 
qof_session_error, if necessary.

> > > 4) You change a lot of style structure which makes it harder to read
> > > your patches.  :)
> > Only in GncInvoice did I inadvertently change the spacing in the
> > declarations
> > - that was only 5 lines.
> >
> > :-(
>
> There were a number of places where you changed:
>
>    foo *      fooobj;
> to
>    foo       *fooobj;

That was on those 5 lines in GncInvoice.c - honest!

> There's no reason to do that...   IMHO...  It's just a style thing. 

OK. Sorry.

-- 

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-devel/attachments/20050510/0b35d750/attachment.bin