I don't want to pick on Teodor in particular, because I've seen other
people do this too, but which of these functions do you find more

          ObjectIdGetDatum(name2cfgId((text *) PG_GETARG_POINTER(0), false)),

    text       *cfgname = PG_GETARG_TEXT_P(0);
    text       *txt = PG_GETARG_TEXT_P(1);
    Oid         cfgId;

    cfgId = name2cfgId(cfgname, false);

The main drawback to the V1-call-convention function call mechanism,
compared to ordinary C functions, is that you can't instantly see what
the function arguments are supposed to be.  I think that good coding
style demands ameliorating this by declaring and extracting all the
arguments at the top of the function.  The above example is bad enough,
but when you have to dig through a function of many lines looking for
GETARG calls in order to know what arguments it expects, it's seriously
annoying and unreadable.

And another thing: use the correct extraction macro for the argument's
type, rather than making something up on the fly.  Quite aside from
helping the reader see what the function expects, the first example
above is actually *wrong*, as it will crash on toasted input.

OK, I'm done venting ... back to patch-fixing.

                        regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
       subscribe-nomail command to [EMAIL PROTECTED] so that your
       message can get through to the mailing list cleanly

Reply via email to