ofx and generic import: GUI, callbacks

[Benoit Grégoire]

> > Unless you mean for the first page to have one line per transaction, I
> > don't see how you could handle it on your previous druid page.
>
> As for the GUI for Destination Account matching/choosing, I absolutely
> have a listview in mind. The listview would have one line per
> transaction. Each line shows some details for the transaction (date,
> amount, description, ...) *and* the destination account. The destination
> account is either the automatically matched one, or empty by default, or
> some fixed account by default. The user then can click on each line's
> account to choose another account. In your example, the user would see
> one line for "WalMart Date xx Description xx Amount xx" and another line
> for "Walmart Date yy Description yy Amount yy" and can choose whatever
> destination account he/she considers appropriate.
>
> All our discussion about "automatic destination account matching" IMHO
> is *only* about the suggested default. The user should *always* have the
> choice to choose some other destination account (and, BTW, this is the
> only way to learn the automatic matches in the first place).

Ok, that would technically work.  But the user would be presented with the 
exact same list of transactions twice.  This just duplicates work for the 
user.  He would have to first select or double check the auto-selection of 
the destination account (possibly by finding his sales receipt for several of 
the transactions) for every transaction.  Then once done he would have to 
start again (again possibly refering again to paper receipts) to see if the 
transactions have to clear an existing transaction or be added.

Even worse, if he decides to user CLEAR on a significant fraction of the 
transaction, a significant portion of his previous work is completely wasted. 

In short, I completely disagree that destination acount matching is a separate 
process from transaction matching for the user.  There both part of the 
intellectual process "Here's is a downloaded transaction, what should I do 
with it".   

I'll send my proposed GUI in an other email.

> I agree that you will need to pass the pointer of the GenericImporter
> into each of your ofx_proc_transaction_cb (@§%$! could you please quote
> the correct name of your functions? Having to guess the right name on my
> own doesn't really increase the fun of understanding other people's
> code.) In fact you need mechanisms like that quite often. The usual
> implementation of that (in C !) is that a callback function has an
> additional parameter,
>
> foo_callback(something *previous_args, void *user_data)
>
> where the programmer of callback knows the type of what user_data points
> to and therefore can simply cast the pointer to the right type. Voilà,
> now you can pass anything to into your callback.

Yes, but the callback is called by libofx, not by the generic importer.  Even 
if it did support passing of user data, it would not help, since the pointer 
would have to be passed to libofx at callback registration time (during 
module initialisation) when the matcher instance does not yet exist.  So i'll 
have to put it in a global anyway.

P.S.:  sorry about the function name mistake :(  

> Speaking of passing into the callback: How does your libOFX know which
> functions it is supposed to call as callback? From inspecting
> src/import-export/ofx/gnc-ofx-import.c I can't tell. Actually since you
> don't use the function names (e.g. ofx_proc_transaction_cb) anywhere
> else in the gnucash code except in its definition it seems that libOFX
> simply calls those functions by their name and hopes that they exist
> "somewhere"?!? This is really not what I would consider good coding
> practice. Because it means that from only inspecting the gnucash code, I
> cannot know at all which functions are going to be called from libOFX.
> Also, how can libOFX know that those functions are really the right ones
> it wanted to call? Also, what if you have several possible usable
> callback functions and have to choose one out of them at runtime? The
> usual (correct?) way of callbacks in C from another object goes like
> this: At some place, the address of the cb function is passed to that
> other object:
>
> other_object_set_cb ( & my_callback_function );

I was inspired by the OpenSP generic interface, at the time it was my first 
API and I tought it was standard practice (Especially since it is written by 
James Clark http://www.jclark.com/bio.htm) .  I now know this isn't the case 
and fully agree with you, but proper callback registration is not a trivial 
undertaking and will have to wait untill the next round of major interface 
changes in libofx.

> > Which implies that we will also have to define a
> > gnc_import_importer_complete_cb(GenericImporter *importer) to tell the
> > OFX module that the matcher has
> > been closed by the user and should now be destroyed.
>
> No you don't need that. Please first implement the correct callback
> mechanism as described above. Otherwise we'll pretty much talk about
> different things in our discussion.
>
> If you want to see how this works in practice, have a look at
> src/import-export/hbci/hbci-interaction.c. At the very end, in the call
> to HBCI_InteractorCB_new, all the callback functions are set. Also, the
> very last argument to that function is precisely that user_data thingy
> mentioned above. You can see how the user_data is cast to the actual
> type in each of the actual callback functions.

I still don't understand.  Could you tell me by example how HBCI will know 
when it is safe to g_free the transaction matcher?  I think it's going to 
have to free itself, leading to dangling pointers.

> my comments above: If there is a C function, and it is used from
> somewhere else, then for the shoot of it there needs to be a header. And
> by convention (which I'm now asking you to adhere to) the header file
> has to have the same name as the implementation file, cause otherwise
> we'll have to search around like stupid. If those headers files end up
> to be totally independent from each other, requiring the protocol-module
> to include a whole lot of headers,  then this is just the representation
> of what's going on. If those headers end up to depend on each other like
> crazy, then the protocol-module will include only the "last leaf" of the
> dependency tree and by this will have all headers included.

All right, I will follow the GnuCash convention.

> I totally understand your point. Nevertheless, after I thought about it
> for some more time, I came to the conclusion that the *real* point here
> are the objectives: What objectives do we have at Gnucash? The objective
> is to deliver a good application to the user. Therefore it's quite
> pointless to have a discussion about API design here -- if at the end of
> the day the user experience is satisfying, I don't give a darn shoot
> about whatever API was used to achieve this. Really. With the obvious
> exception that for the important data types (Transaction, Split,
> Account) we have to get them right in the first place. But an "API for
> the generic importer"? Honestly, the design of that API doesn't interest
> me at all as long as the user experience is fine. Which, in your case,
> by definition couldn't be tested until after you started coding. Which,
> in turn, explains why "negative feedback" hasn't been brought up earlier.

Yes, we both have the same objective:  to deliver a good application for the 
user, but not the same point of view on how to attain it.  We obviously need 
more developpers (off course every project could use more developpers, but I 
am under the impression that we barely have enough to manage the current 
codebase).  And I think the barrier to entry for new developpers is far too 
high.

What I am trying to do is to make sure that the next developper that comes in 
to work on import functionnality will be able to focus on his own 
functionnality, and not have to re-learn the hard way all that I had to 
learn.  When I first announced I was going to write OFX import, and asked 
what module I could use as a code example, I was told "There is the QIF 
import module that is not a good example and holds together with small bits 
of tape and good intentions, and there is this great new importer code whose 
design is much better.  Unfortunately no one knows anything about it."

So I've spent far more time figuring out gnucash internals than implementing 
the features I was working on.  If it wasn't for the fact that this was once 
my end of degree project, I WOULD have abandoned even if by that time I had 
what was supposed to be the hard part done (or at least the part where others 
failed before me): reliably parsing ofx files.  And I was working on a major 
and complex protocol (OFX).  I don't think someone working on CSV or 
supporting a simple handheld format would ever go thru that much effort.  Off 
course it didn't help that I didn't know anything about glib, gtk or scheme 
at that time.  But one shouldn't need to master those to be able to write a 
protocol module.   

I agree that the actual API is relatively unimportant as long that it works 
and is easy to understand.  But it has to exist.  What you are saying sound's 
to me like "Let's not bother with internal APIs, since the time lost writing 
and documenting them will never offset the time lost by core developpers that 
could have been better spent writing new features faster".  That may work 
fine for small projects, but won't help us in the long run if we can't 
attract casual developpers.

For a big project like GnuCash there has to be explicit groups of headers used 
for certain tasks, and they must at the very least be listed along with their 
function.  A new developper coming in GnuCash will initially want and need to 
understand only few of the following:
-Engine interface
-Interface to create and load a module
-Interface to add menu entry and user prefs
-Interface to write reports
-Common utility gunctions used for GUI hacking
-Backend interface
-Scheduled transaction interface
-Small buisness?
Much of that exists, but isn't packaged in a easy to read group of header 
files.

> Sounds good. BTW I might still get into some "coding flash" this weekend
>   and quickly implement the destination matching. In that case it would
> already be in 1.8.0.

Gess we have to get on IRC then...