Re: PgJDBC: code reformat

On 27 December 2015 at 16:01, Gavin Flower <GavinFlower(at)archidevsys(dot)co(dot)nz>
wrote:

> On 28/12/15 04:01, Vladimir Sitnikov wrote:
>
>> I did try reformatting with "opening braces on new line".
>> It turned out it is incompatible with checkstyle:
>> https://travis-ci.org/pgjdbc/pgjdbc/jobs/98976190
>>
>> There are 17 "... have incorrect indentation level" failures.
>> Most of which are due to enum initialization in
>>
>> https://github.com/pgjdbc/pgjdbc/blob/197175039068446a15c30d2b5e949f1eae08515d/pgjdbc/src/main/java/org/postgresql/hostchooser/HostRequirement.java#L16-L29
>>
>> I have just two requirements:
>> 1) IDEA's autoformat should be able to produce the desired style
>> (minimal modifications are bearable)
>> 2) The style should be enforced, in other words, checkstyle (or
>> whatever tools is used) should be able to catch most of the styling
>> issues.
>>
> Agreed!

+1

>
>> Compare this one:
>>
>> https://github.com/pgjdbc/pgjdbc/blob/197175039068446a15c30d2b5e949f1eae08515d/pgjdbc/src/main/java/org/postgresql/jdbc/PgResultSet.java#L2462-L2512
>> with this one:
>> https://github.com/pgjdbc/pgjdbc/blob/197175039068446a15c30d2b5e949f1eae08515d/pgjdbc/src/main/java/org/postgresql/core/v3/QueryExecutorImpl.java#L264-L311
>>
>> Do you see how QueryExecutorImpl.java#L264-L311 is much readable?
>>
> No. Possibly I'm not seeing the code you want me too?
>
Same problem here, one of them shows comments. I believe the first one.

>
> I would add more blank lines for readability, but would eliminate blank
> lines after opening brackets and before closing brackets.
>
> Also would simplify lines to ease readability and improve ability to
> debug, eg
>
>
So there are very good reasons in Java not to do what you are proposing
below. Mostly because it creates objects which have to be garbage collected.
While I agree that it is more readable. I think driver code has a
responsibility to be as curt as possible with respect to creating objects.

> Would rewrite:
> ((V3ParameterList) parameterList).checkAllParametersSet();
> as
> V3ParameterList v3ParameterList =(V3ParameterList) parameterList;
> v3ParameterList.checkAllParametersSet();
>
>
> and would rewrite:
> if ((flags &QueryExecutor.QUERY_SUPPRESS_BEGIN)
> !=0||protoConnection.getTransactionState()
> !=ProtocolConnection.TRANSACTION_IDLE)
> as
> boolean abc =(flags &QueryExecutor.QUERY_SUPPRESS_BEGIN) !=0;
> boolean xyz = protoConnection.getTransactionState()
> !=ProtocolConnection.TRANSACTION_IDLE;
> if (abc||xyz)
> (except I would use more meaningful names for 'abc' & 'xyz' above!)
>
> I also only ever have one return per method, as that makes it easier to:
> (1) understand
> (2) maintain (add new code and/or change existing code)
> (3) debug
>
>
>> PgResultSet#getFastLong is very hard to follow no matter which way you
>> format the braces.
>> I believe, "readability" comes from proper segmentation (code blocks
>> vs methods) and proper variable naming.
>>
>

Dave Cramer

davec(at)postgresintl(dot)com
www.postgresintl.com