Re: PgJDBC: code reformat - Mailing list pgsql-jdbc

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!

>
> As Garvin/Dave/Markus vote for "braces on new line", would you please
> suggest how to fix checkstyle errors?
> For instance:
> /home/travis/build/pgjdbc/pgjdbc/pgjdbc/src/main/java/org/postgresql/jdbc/PgResultSet.java:2439:
> error: 'object def lcurly' have incorrect indentation level 6,
> expected level should be one of the following: 2, 4.
>
https://github.com/pgjdbc/pgjdbc/blob/197175039068446a15c30d2b5e949f1eae08515d/pgjdbc/src/main/java/org/postgresql/jdbc/PgResultSet.java#L2439
Never used CheckStyle.

>
> PS.
> I do not care where the braces are.
> However, this makes me cry:
>
https://github.com/pgjdbc/pgjdbc/blob/197175039068446a15c30d2b5e949f1eae08515d/pgjdbc/src/main/java/org/postgresql/Driver.java#L353-L373
> There are probably better ways of writing that, but those "new lines
> for opening brackets" make it much worse.
>
>
> PPS.
>> The other style looks cluttered, and is harder to see
> Gavin,
> I feel your pain, however I still think you are trying to solve "badly
> written code" with "reformat code" approach.
> That just does not work.
It definite works!  At least the way I structure my code.  I will
refactor my code if it gets to look complicated, often after I have
understood the problem I'm solving better.  I use the same style of
coding for one off explorations for personal use, as I do for production
code.

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

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

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.
Good code structuring and naming conventions are essential!

Absolutely NO coding style can make up for bad code structuring and
naming conventions!!!

If my nested if statements start getting unwieldy, I will move parts
into separate methods - as this makes it easier to read and the naming
of the methods help document what I'm trying to do.  Plus I will break
down complicated conditionals into pre-calculated booleans (see example
above) for better readability.

Sometimes I'll have a method with only a few lines of code, if it is
invoked from multiple places and has strategic significance.  The
optimising complier will in-line code that gets invoked frequently, so
no loss in performance doing so.

I spend far more time reading my code than writing it, so I'm prepared
to put much more effort into writing and reformatting code, than most
people appear to do.

>
> Vladimir