On 18/04/15 20:35, Dmitry Dolgov wrote:
Sorry for late reply. Here is a slightly improved version of the patch
with the new `h_atoi` function, I hope this implementation will be more
appropriate.


It's better, but a) I don't like the name of the function b) I don't see why we need the function at all given that it's called from only one place and is effectively couple lines of code.

In general I wonder if it wouldn't be better to split the replacePath into 3 functions, one replacePath, one replacePathObject, replacePathArray and call those Object/Array ones from the replacePath and inlining the h_atoi code into the Array one. If this was done then it would make also sense to change the main if/else in replacePath into a switch.

Another thing I noticed now when reading the code again - you should not be using braces when you only have one command in the code-block.

--
 Petr Jelinek                  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


--
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