Hello,
>> - I would suggest to avoid "continue" within a loop so that the code is
>> simpler to understand, at least for me.
>
> I personally find the code easier to read with the continue.
Hmmm. I had to read the code to check it, and I did it twice. The point is
that there is 3 exit points instead of 1 in the block, which is not in
itself very bad, but:
for(...) { if (ccc) xxx; ... foo++; }
It looks like "foo++" is always executed, and you have to notice that xxx
a few lines above is a continue to realise that it is only when ccc is
false. This is also disconcerting if it happens to be the "rare" case,
i.e. here most of the time the char is not '%', so most of the time foo is
not incremented, although it is a the highest level. Also the code with
continue does not really put forward that what is counted is '%' followed
by a non '%'. Note that the corresponding execution code
(pgplsql/src/pl_exec.c) uses one continue to get rid of the second '%',
which is quite defendable in that case as it is a kind of exception, but
the main condition remains a simple if block. Final argument, the
structured version is shorter than the unstructured version, with just the
two continues removed, and one negation also removed.
> to also turn the ereport()s into elog()s since the user should never see
> them.
I'm not sure why elog is better than ereport in that case: ISTM that it is
an error worth reporting if it ever happens, say there is another syntax
added later on which is not caught for some reason by the compile-time
check, so I would not change it.
--
Fabien.
