Macros that affect program flow. (was Re: temporary absence)

[Derek Atkins]

The point of a macro is to simplify the process of programming.
In particular, it's supposed to allow you to replace repetitive
code.

Pretty much EVERY INSTANCE of gncFooBeginEdit() and gncFooCommitEdit()
looks EXACTLY THE SAME!  It seems really stupid to me to make every
object have to call functions, check the results, and then return
itself when a central macro can do it for you.

IMHO this is a DOCUMENTATION problem, not a clarity-of-API issue.
The API is quite clear -- the MACRO exists to simplify your life.
The user of the QOF API need not use the macro if they don't want to,
but it most certainly simplifies your life when you do use it.

Changing the macro not to return would IMNSHO make the developers
life more difficult.  In my particular case I would just write
YET ANOTHER MACRO to use in gnucash, so we don't have duplicated
code in every single qof object we define.  So why not just leave
that macro, which would get written anyways, in QOF?

-derek

Neil Williams <linux at codehelp.co.uk> writes:

> On Sunday 26 February 2006 3:17 am, you wrote:
>> > Apart from that and the changes committed to trunk, does anyone have any
>> > more code out-of-tree that should go into QOF 0.6.2?
>>
>> Yes.  I've got some changes related to QOF_COMMIT_EDIT_PART2. 
>
> OK, I've got r13390 and r13391 (guid.c). PROBLEM:
>
> The macros in qof-be-utils.h are awkward because they (all) change programme 
> flow. We can't redefine QOF_BEGIN_EDIT as a call to qof_begin_edit because 
> functions that use QOF_BEGIN_EDIT rely on it calling return if the instance 
> is NULL or edit_level < 1. I don't like macros that work like that and I 
> would like to deprecate each of these macros and force the use of the 
> functions in libqof2.
>
> We cannot redefine QOF_COMMIT_EDIT_PART2 because the last call in the macro is 
> 'return':
> -  if ((inst)->do_free) {                                         \
> -     (on_free)(inst);                                            \
> -     return;                                                     \
> -  }  
>
> Your code simply returns from the qof_commit_edit_part2 function, it does NOT 
> return from the function that called the original macro. To do that, existing 
> code would have to check the return value of the function that is behind the 
> macro. Not impossible, but not required by the current API either. That's an 
> API change.
>
> I got bitten by this problem when I first tried to use a function in place of 
> QOF_BEGIN_EDIT.
>
> IF the macro is NOT followed by any further code in the calling function, this 
> would be OK but there is nothing to say that this is the case (or has to be 
> the case). The current API (rightly or wrongly) allows programmes to call 
> QOF_COMMIT_EDIT_PART2 and then continue processing the instance. Such code is 
> assuming that if the instance was freed as part of the commit that the macro 
> will have returned OUT of the function itself. A function replacing 
> QOF_COMMIT_EDIT_PART2 will still free the instance but now processing 
> continues into the rest of the function which is not expecting a freed 
> instance to be possible. Even having a LEAVE macro that references the now 
> freed instance would cause a seg fault.
>
> Derek - I would v.v.v.much like to deprecate all the macros in qof-be-utils.h 
> precisely because of these problems. They cause no end of confusion and 
> hidden assumptions. I know you wanted these as macros but I think the API is 
> too confusing. IMHO, the pros of the macros do not outweigh their problems. 
> The functions are easier to use - particularly because they can be used in 
> functions that return a value. To me, macros should not determine whether or 
> not I can return a value from an internal function.
>
> In fact, once the macros are deprecated (i.e. moved into deprecated.h and 
> tagged in doxygen), qof-be-utils.h can be removed and the function 
> declarations made part of qofbackend.h (where I think new users of libqof 
> would expect to find them).
>
> Chris: We need to replace the macro and have the function as an alternative, 
> *not* a replacement. It matters not whether gnucash has a problem with the 
> function, it matters that the API is changed by the change in programme flow 
> generated within the old macro but not implementable in the function version.
>
>> I have 
>> verified that this macro is not used outside of GnuCash. 
>
> That's not the whole story for an API issue - a macro in an API must retain 
> it's arguments and behaviour, especially it's use of 'return;'.
>
> i.e. code that uses the old macro must still function the same way. The macro 
> is still part of the API (until libqof2). We need the macro replaced, for 
> now. I don't like it, but that's the way it is until libqof2.
>
> If there is agreement to deprecate all these macros, then the functions will 
> replace all the macros in libqof2. Until then, libqof1 requires the old macro 
> to be in place.
> :-(
>
> -- 
>
> Neil Williams
> =============
> http://www.data-freedom.org/
> http://www.nosoftwarepatents.com/
> http://www.linux.codehelp.co.uk/
>
> _______________________________________________
> gnucash-devel mailing list
> gnucash-devel at gnucash.org
> https://lists.gnucash.org/mailman/listinfo/gnucash-devel

-- 
       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
       Member, MIT Student Information Processing Board  (SIPB)
       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
       warlord at MIT.EDU                        PGP key available