I personally prefer not to break lines with few parameters.
It just feels unnecessarily clumsy to parse the breaks if there are only
two or three arguments with short names.
So +1
- for a hard line length limit
- allowing arguments on the same line if below that limit
- with consistent argument breaking when that length is exceeded
- developers can break before that if they feel it helps with readability.
This should be similar to what we have, except for enforcing the line
length limit.
I think our Java guide originally suggested 120 characters line length, but
we can reduce that to 100 if a majority argues for that, but it is separate
discussion.
We uses shorter lines in Scala (100 chars) because Scala code becomes
harder to read faster with long lines.
On Mon, Aug 19, 2019 at 10:45 AM Andrey Zagrebin <and...@ververica.com>
wrote:
> Hi Everybody,
>
> Thanks for your feedback guys and sorry for not getting back to the
> discussion for some time.
>
> @SHI Xiaogang
> About breaking lines for thrown exceptions:
> Indeed that would prevent growing the throw clause indefinitely.
> I am a bit concerned about putting the right parenthesis and/or throw
> clause on the next line
> because in general we do not it and there are a lot of variations of how
> and what to put to the next line so it needs explicit memorising.
> Also, we do not have many checked exceptions and usually avoid them.
> Although I am not a big fan of many function arguments either but this
> seems to be a bigger problem in the code base.
> I would be ok to not enforce anything for exceptions atm.
>
> @Chesnay Schepler <ches...@apache.org>
> Thanks for mentioning automatic checks.
> Indeed, pointing out this kind of style issues during PR reviews is very
> tedious
> and cannot really force it without automated tools.
> I would still consider the outcome of this discussion as a soft
> recommendation atm (which we also have for some other things in the code
> style draft).
> We need more investigation about how to enforce things. I am not so
> knowledgable about code style/IDE checks.
> From the first glance I also do not see a simple way. If somebody has more
> insight please share your experience.
>
> @Biao Liu <mmyy1...@gmail.com>
> Line length limitation:
> I do not see anything for Java, only for Scala: 100 (also enforced by build
> AFAIK).
> From what I heard there has been already some discussion about the hard
> limit for the line length.
> Although quite some people are in favour of it (including me) and seems to
> be a nice limitation,
> there are some practical implication about it.
> Historically, Flink did not have any code style checks and huge chunks of
> code base have to be reformatted destroying the commit history.
> Another thing is value for the limit. Nowadays, we have wide screens and do
> not often even need to scroll.
> Nevertheless, we can kick off another discussion about the line length
> limit and enforcing it.
> Atm I see people to adhere to a soft recommendation of 120 line length for
> Java because it is usually a bit more verbose comparing to Scala.
>
> *Question 1*:
> I would be ok to always break line if there is more than one chained call.
> There are a lot of places where this is only one short call I would not
> break line in this case.
> If it is too confusing I would be ok to stick to the rule to break either
> all or none.
> Thanks for pointing out this explicitly: For a chained method calls, the
> new line should be started with the dot.
> I think it should be also part of the rule if forced.
>
> *Question 2:*
> The indent of new line should be 1 tab or 2 tabs? (I assume it matters only
> for function arguments)
> This is a good point which again probably deserves a separate thread.
> We also had an internal discussion about it. I would be also in favour of
> resolving it into one way.
> Atm we indeed have 2 ways in our code base which are again soft
> recommendations.
> The problem is mostly with enforcing it automatically.
> The approach with extra indentation also needs IDE setup otherwise it is
> annoying
> that after every function cut/paste, e.g. Idea changes the format to one
> indentation automatically and often people do not notice it.
>
> I suggest we still finish this discussion to a point of achieving a soft
> recommendation which we can also reconsider
> when there are more ideas about automatically enforcing these things.
>
> Best,
> Andrey
>
> On Sat, Aug 3, 2019 at 7:51 AM SHI Xiaogang <shixiaoga...@gmail.com>
> wrote:
>
> > Hi Chesnay,
> >
> > Thanks a lot for your reminder.
> >
> > For Intellij settings, the style i proposed can be configured as below
> > * Method declaration parameters: chop down if long
> > * align when multiple: YES
> > * new line after '(': YES
> > * place ')' on new line: YES
> > * Method call arguments: chop down if long
> > * align when multiple: YES
> > * take priority over call chain wrapping: YES
> > * new line after '(': YES
> > * place ')' on new line: YES
> > * Throws list: chop down if long
> > * align when multiline: YES
> >
> > As far as i know, there does not exist standard checks for the alignment
> of
> > method parameters or arguments. It needs further investigation to see
> > whether we can validate these styles via customized checks.
> >
> >
> > Biao Liu <mmyy1...@gmail.com> 于2019年8月2日周五 下午4:00写道：
> >
> > > Hi Andrey,
> > >
> > > Thank you for bringing us this discussion.
> > >
> > > I would like to make some details clear. Correct me if I am wrong.
> > >
> > > The guide draft [1] says the line length is limited in 100 characters.
> > From
> > > my understanding, this discussion suggests if there is more than 100
> > > characters in one line (both Scala and Java), we should start a new
> line
> > > (or lines).
> > >
> > > *Question 1*: If a line does not exceed 100 characters, should we break
> > the
> > > chained calls into lines? Currently the chained calls always been
> broken
> > > into lines even it's not too long. Does it just a suggestion or a
> > > limitation?
> > > I prefer it's a limitation which must be respected. And we should
> always
> > > break the chained calls no matter how long the line is.
> > >
> > > For a chained method calls, the new line should be started with the
> dot.
> > >
> > > *Question 2:* The indent of new line should be 1 tab or 2 tabs?
> Currently
> > > there exists these two different styles. This rule should be also
> applied
> > > to function arguments.
> > >
> > > BTW, big +1 to options from Chesnay. We should make auto-format
> possible
> > on
> > > our project.
> > >
> > > 1.
> > >
> > >
> >
> https://docs.google.com/document/d/1owKfK1DwXA-w6qnx3R7t2D_o0BsFkkukGlRhvl3XXjQ/edit#
> > >
> > > Thanks,
> > > Biao /'bɪ.aʊ/
> > >
> > >
> > >
> > > On Fri, Aug 2, 2019 at 9:20 AM SHI Xiaogang <shixiaoga...@gmail.com>
> > > wrote:
> > >
> > > > Hi Andrey,
> > > >
> > > > Thanks for bringing this. Personally, I prefer to the following style
> > > which
> > > > (1) puts the right parenthese in the next line
> > > > (2) a new line for each exception if exceptions can not be put in the
> > > same
> > > > line
> > > >
> > > > That way, parentheses are aligned in a similar way to braces and
> > > exceptions
> > > > can be well aligned.
> > > >
> > > > *public **void func(*
> > > > * int arg1,*
> > > > * int arg2,*
> > > > * ...
> > > > *) throws E1, E2, E3 {*
> > > > * ...
> > > > *}*
> > > >
> > > > or
> > > >
> > > > *public **void func(*
> > > > * int arg1,*
> > > > * int arg2,*
> > > > * ...
> > > > *) throws
> > > > * E1,
> > > > * E2,
> > > > * E3 {*
> > > > * ...
> > > > *}*
> > > >
> > > > Regards,
> > > > Xiaogang
> > > >
> > > > Andrey Zagrebin <and...@ververica.com> 于2019年8月1日周四 下午11:19写道：
> > > >
> > > > > Hi all,
> > > > >
> > > > > This is one more small suggestion for the recent thread about code
> > > style
> > > > > guide in Flink [1].
> > > > >
> > > > > We already have a note about using a new line for each chained call
> > in
> > > > > Scala, e.g. either:
> > > > >
> > > > > *values**.stream()**.map(...)**,collect(...);*
> > > > >
> > > > > or
> > > > >
> > > > > *values*
> > > > > * .stream()*
> > > > > * .map(*...*)*
> > > > > * .collect(...)*
> > > > >
> > > > > if it would result in a too long line by keeping all chained calls
> in
> > > one
> > > > > line.
> > > > >
> > > > > The suggestion is to have it for Java as well and add the same rule
> > > for a
> > > > > long list of function arguments. So it is either:
> > > > >
> > > > > *public void func(int arg1, int arg2, ...) throws E1, E2, E3 {*
> > > > > * ...*
> > > > > *}*
> > > > >
> > > > > or
> > > > >
> > > > > *public **void func(*
> > > > > * int arg1,*
> > > > > * int arg2,*
> > > > > * ...)** throws E1, E2, E3 {*
> > > > > * ...*
> > > > > *}*
> > > > >
> > > > > but thrown exceptions stay on the same last line.
> > > > >
> > > > > Please, feel free to share you thoughts.
> > > > >
> > > > > Best,
> > > > > Andrey
> > > > >
> > > > > [1]
> > > > >
> > > > >
> > > >
> > >
> >
> http://mail-archives.apache.org/mod_mbox/flink-dev/201906.mbox/%3ced91df4b-7cab-4547-a430-85bc710fd...@apache.org%3E
> > > > >
> > > >
> > >
> >
>
