2014-08-12 15:09 GMT+02:00 Fabien COELHO <coe...@cri.ensmp.fr>:

>
> 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.
>
>
one note: this patch can enforce a compatibility issues - a partially
broken functions, where some badly written RAISE statements was executed
newer.

I am not against this patch, but it should be in extra check probably ?? Or
we have to documented it as potential compatibility issue

Regards

Pavel


>  --
> Fabien.
>
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

Reply via email to