[Gnucash-changes] r13390 - gnucash/trunk/lib/libqof/qof - Implement QOF_COMMIT_EDIT_PART2 as a function instead of a macro.

[Chris Shoemaker]

On Tue, Feb 28, 2006 at 12:43:42PM -0500, Derek Atkins wrote:
> Chris Shoemaker <c.shoemaker at cox.net> writes:
> 
> > On Sun, Feb 26, 2006 at 11:57:19AM -0500, Derek Atkins wrote:
> >> This patch could break a lot of code in lots of places because there
> >> is code that expects that QOF_COMMIT_EDIT_PART2() could return
> >> (exiting the /caller/ of the macro), but the new macro wont return
> >> out of the caller anymore.
> >
> > Yes, this change would certainly break certain uses of this macro.
> > However, of all the uses, only one falls in to that case.  I'll fix
> > that one case to use the function directly.
> 
> Like I said earlier, the point of the macro was to reduce duplicated
> code.  Nowhere is there any rule against macros that affect program
> flow.  It's perfectly legal, and in many cases quite convenient, to
> have a macro that causes the current function to return.

:) I'm betting you've never been caught be a bug involving
control-flow hidden behind a macro.  In my experience, it only takes
once until someone decides that they will never, ever write one.

> 
> It's certainly better that having duplicated code in 20 places.

I've never seen a case where the only choices were between a
control-flow macro and any significant duplicated code.

> 
> > BTW, eliminating a macro that affects control flow wasn't my
> > motivation, but it is a fringe benefit.  It's *horrible* practice and
> > we fortunately only depend(ed) on it in one place.
> 
> Why is it a "*horrible* practice"?  I'd rather have convenience macros
> that affect program flow than have to change code in twenty different
> places when some particular behavior changes.

The reasons are many.  It's better to just reference one of many
public rants on this topic.  Here's one:
http://www.coker.com.au/~russell/ccode/macros.html

I would say that in mature development communities this particular
practice is so commonly recognized as poor that it's practically
become a cliche.  I've seen many a poor unsuspecting new developers
get thoroughly roasted for writing such a macro, which is unfortunate
because it's one of those things that appears more clever than harmful
to the person writing it.

It's just one of those things that falls into the category of "small
convenience for person writing the code, huge inconvenience for anyone
reading the code," which, of course, is perfectly acceptable, only if
one doesn't intend for the code to ever be read.

<aside>
BTW, this is only slightly less horrible than macros that use local
variables from their calling context.  The general rule is, "If it
looks like a function, it sure better behave like a function." (*)
</aside>

-chris

(*) Some communities even enforce this to the point of demanding that
you can take the address of function-like macros.  At first blush,
this appears ridiculous, but it turns out to be quite simple because C
allows there to be a function and a macro with the exact same name.
The compiler will use the macro in a calling context, avoiding a
function call, and the function in an address context.  So, you just
define the function to call the macro, and you get the best of both
worlds, with only one symbol.  Incidentally, this seems to have become
less common since the introduction of 'inline' to C99.