New QOF type, QOF_TYPE_COLLECT and recursive entity copying
Neil Williams
linux at codehelp.co.uk
Tue May 10 04:58:17 EDT 2005
On Monday 09 May 2005 9:46 pm, Derek Atkins wrote:
> Neil,
>
> A couple of comments on this patch. I'll note that I haven't looked very
> closely at it, but in my cursory examination I see a number of issues:
I committed this too early - there were bits that were unfinished and bits
that should have been cleaned up. My bad.
However, I simply don't understand your objection to GncEntry being a
QOF_TYPE_COLLECT from GncInvoice - see later.
> 1) You introduce a number of memory leaks. For example, when you just
> reset the address pointer to a passed-in object you potentially leak
> memory.
Is this the correct solution?
void
qofCustomerSetAddr (GncCustomer *cust, QofEntity *addr_ent)
{
GncAddress *addr;
if(!cust || !addr_ent) { return; }
addr = (GncAddress*)addr_ent;
if(addr == cust->addr) { return; }
if(cust->addr != NULL) { gncAddressDestroy(cust->addr); }
cust->addr = addr;
}
> You have similar issues in places like qofOwnerGetType() where I
> really don't think you should be returning g_strdup'd strings when the data
> is constant.
Fixed. Thanks.
> 2) You add a number of APIs that really don't belong. There is absolutely
> no reason to export the invoices list of entries as a QofCollection (or a
> KVP frame).
The KVP frame was redundant, it wasn't meant to be committed. The
QofCollection, however, I need.
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.
QOF_TYPE_COLLECT (QofCollection) is the new QOF type, it's not a regular
collection, it's a secondary collection that is used to pass references to
many entities of the same type in one parameter. It doesn't contain all
entities from the book, it contains all entities associated with the
parameter. In this case, it's every (QofEntity*)GncEntry in the GList* of the
specific GncInvoice*. The collection is written out as a series of tags and
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.
QOF_TYPE_COLLECT is being used, in this example, as a "typed list". It's a
block of QofEntity pointers with a guarantee that all entities are of the
same e_type. In the backend, this is converted to a block of pointers to
QofEntityReference that contains the GUID and the type of each. This is then
written to QSF. I cannot simply write out the GUID using the list, QOF needs
the type, hence a collection.
> I'd be less concerned if this were a private API -- but it
> certainly does not belong in the .h file and exported from the module.
? This is the only problem that remains.
?? 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?
What have I missed? QOF is working from the invoice down - it locates the
Customer etc. via the GncOwner Collection, it locates the billterms using a
one-to-one reference. The invoice owns the entries (yes?), so whether the
invoice has been posted or not, QOF should be able to access the entries from
the invoice object. How else can I create a new invoice - with entries - in
from QOF?
The business objects are written to v2 XML via Scheme and it's complete
mumbo-jumbo to me (even the generated C code). I can't tell how the Invoice
and the Entry are currently processed from the v2 XML and there's absolutely
*no* documentation.
:-(
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.
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.
> 3) I don't see why you need a "qof_temp" storage in the GncOwner object.
> It seems superfluous.
I changed qofOwnerSetEntity and didn't twig that qofOwnerSetType and
qofOwnerSetOwner were left hanging without much to do. Removed.
> 4) You change a lot of style structure which makes it harder to read your
> patches. :)
Now there's two things going on there, first the majority of all those changes
are Doxygen fixes. Anywhere that I've changed:
-/** @name Create/Destroy Functions */
-/** @{ */
+/** @name Create/Destroy Functions
+ @{ */
Those were generating horrible artefacts in the Doxygen output - prefixing
function declarations in the named block with * - confusion was inevitable.
const char* myfunc(); // OK
*const char* myfunc(); // NOT OK
Only in GncInvoice did I inadvertently change the spacing in the declarations
- that was only 5 lines.
:-(
--
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/20050510/e681d348/attachment.bin
More information about the gnucash-patches
mailing list