Re: Updating our Code Contribution/Style Guide
On Tue, 15 Mar 2022 at 11:46, Ruslan Fomkin wrote:
> …
> I support Jacek’s request to have each argument on a separate line when
> they are many and need to be placed on multiple lines. For me it takes less
> effort to grasp arguments on separate lines than when several arguments are
> combined on the same line. IMHO the root cause is having too many
> arguments, which is common issue for non-OOP languages.
>
I support this too. It has always bugged me that by having the first
parameter not on a newline, then when the method (or variable assigned)
name changes it causes all subsequent wrapped parameter lines have to be
re-indented, which leads to more noise in, and less readability of, the
patch.
For example,
method(
"a",
"b",
"c"
)
is better than
*method*("a",
"b",
"c"
)
I also agree that several arguments on the one line should be avoided,
that too many method parameters is the problem here.
I would also like to suggest that an operator should always carry on
line wraps. This makes it faster to read the difference between
arguments per line wrapping, and operations over multiple lines.
For example, ensuring it looks like
var = bar == null
? doFoo()
: doBar();
and not
var = bar == null ?
doFoo() :
doBar();
Re: Updating our Code Contribution/Style Guide
Regarding `instance()` method / `instance` field - to clarify my point - we
usually use that in many places. While it is quite easy to access by method
rather than by a field from the beginning, regardless if there is a need
for a mock immediately or not, it would be a much bigger change in terms of
lines/conflicts when we decide to change that later. This is just a
suggestion to make it more testable in our world full of singleton without
mass refactoring.
Obviously, we can formulate the requirement in different words - do have
80%+ coverage of unit tests (not in-jvm dtests) for the new / changed code.
thanks
- - -- --- - -
Jacek Lewandowski
On Sun, Mar 20, 2022 at 8:55 PM Mick Semb Wever wrote:
>
>
> On Tue, 15 Mar 2022 at 11:46, Ruslan Fomkin
> wrote:
>
>> …
>> I support Jacek’s request to have each argument on a separate line when
>> they are many and need to be placed on multiple lines. For me it takes less
>> effort to grasp arguments on separate lines than when several arguments are
>> combined on the same line. IMHO the root cause is having too many
>> arguments, which is common issue for non-OOP languages.
>>
>
>
> I support this too. It has always bugged me that by having the first
> parameter not on a newline, then when the method (or variable assigned)
> name changes it causes all subsequent wrapped parameter lines have to be
> re-indented, which leads to more noise in, and less readability of, the
> patch.
>
> For example,
>
> method(
> "a",
> "b",
> "c"
> )
>
> is better than
>
> *method*("a",
>
>"b",
>
>"c"
> )
>
> I also agree that several arguments on the one line should be avoided, that
> too many method parameters is the problem here.
>
> I would also like to suggest that an operator should always carry on line
> wraps. This makes it faster to read the difference between arguments per line
> wrapping, and operations over multiple lines.
> For example, ensuring it looks like
>
> var = bar == null
>
> ? doFoo()
>
> : doBar();
>
> and not
>
> var = bar == null ?
>
> doFoo() :
>
> doBar();
>
>
>
Re: Updating our Code Contribution/Style Guide
> I support this too… leads to more noise in, and less readability of, the
> patch.
Readability of the patch is not harmed with modern tooling (with whitespace
being highlighted differently to content changes).
Legibility of the code (not patch) should always be preferred IMO. To aid code
comprehension, we should aim for density of useful information for the reader;
wasting a dozen or more lines on zero information density, solely to solve a
problem already handled by modern diff tools, is a false economy.
> I also agree that several arguments on the one line should be avoided, that
> too many method parameters is the problem here.
Method parameters aren’t a problem if they are strongly typed, parameters are
cleanly grouped, and all call-sites utilise approximately the same behaviour.
The alternatives are builders or mutability, the latter of which we broadly
avoid. Builders make code navigation clunkier (creating more indirection to
navigate when searching callers), as well as potentially creating additional
heap pressure and code pollution (both in the call sites and the builder
itself).
Builders are helpful when there are lot of different ways to configure an
object, but the more common case of simply propagating a relevant subset of
existing parameters (plus perhaps a couple of new but required parameters), at
just a handful of equivalent call-sites, they are IMO unhelpful.
Note also that builders have the exact same legibility concerns as
parameter-per-line: a significant amount of screen real-estate is taken up by
scaffolding/noise. This is only useful if the builder call-sites communicate
some unique configuration details about the particular call-site.
> I would also like to suggest that an operator should always carry on line
> wraps
For the ternary operator I agree, however I am less convinced in other cases.
String concatenation is probably cleaner with the opposite norm, so that string
literals are aligned.
From: Mick Semb Wever
Date: Sunday, 20 March 2022 at 19:55
To: dev@cassandra.apache.org
Subject: Re: Updating our Code Contribution/Style Guide
On Tue, 15 Mar 2022 at 11:46, Ruslan Fomkin
mailto:ruslan.fom...@gmail.com>> wrote:
…
I support Jacek’s request to have each argument on a separate line when they
are many and need to be placed on multiple lines. For me it takes less effort
to grasp arguments on separate lines than when several arguments are combined
on the same line. IMHO the root cause is having too many arguments, which is
common issue for non-OOP languages.
I support this too. It has always bugged me that by having the first parameter
not on a newline, then when the method (or variable assigned) name changes it
causes all subsequent wrapped parameter lines have to be re-indented, which
leads to more noise in, and less readability of, the patch.
For example,
method(
"a",
"b",
"c"
)
is better than
method("a",
"b",
"c"
)
I also agree that several arguments on the one line should be avoided, that too
many method parameters is the problem here.
I would also like to suggest that an operator should always carry on line
wraps. This makes it faster to read the difference between arguments per line
wrapping, and operations over multiple lines.
For example, ensuring it looks like
var = bar == null
? doFoo()
: doBar();
and not
var = bar == null ?
doFoo() :
doBar();
Re: Updating our Code Contribution/Style Guide
> I support this too… leads to more noise in, and less readability of, the > patch. > > > > Readability of the patch is not harmed with modern tooling (with > whitespace being highlighted differently to content changes). > > > > Legibility of the code (not patch) should always be preferred IMO. To aid > code comprehension, we should aim for density of useful information for the > reader; wasting a dozen or more lines on zero information density, solely > to solve a problem already handled by modern diff tools, is a false economy. > We are talking about one extra line, not a dozen or more. It also improves the readability of the code IMHO. > > I would also like to suggest that an operator should always carry on > line wraps > > > > For the ternary operator I agree, however I am less convinced in other > cases. String concatenation is probably cleaner with the opposite norm, so > that string literals are aligned. > IMHO it works for string concatenation too. The example that comes to mind is a) *method*( "a", "b", "c" ) b) *method*( "a" + "b" + "c" ) c) *method*( "a" + "b" + "c" ) Once the code gets more real, it is faster to read the difference between (a) and (c) than it (a) and (b).
Re: Updating our Code Contribution/Style Guide
> We are talking about one extra line, not a dozen or more. I think you are confused about the context. The case I was discussing often means 10+ additional lines at each call-site. > Once the code gets more real, it is faster to read the difference between (a) > and (c) This isn’t a great example, as if you are line-wrapping with multiple parameters you should be assigning any computation to a clearly named local variable before passing the result to the constructor (amongst other things). We can perhaps highlight this in the style guide. We also do not only produce multi-line computations in this context. If constructing into a variable (where there is no such ambiguity), it is much easier to parse parameters first without the concatenation operator preceding them. My point is simply that legislating on this kind of detail is a waste of our time, and probably counter-productive. I don’t want to enumerate all the possible ways we might construct multi-line computations. Ternary operators are pretty clear, so maybe we can just agree to define those, and leave the rest to the judgement of the authors? From: Mick Semb Wever Date: Sunday, 20 March 2022 at 20:56 To: dev@cassandra.apache.org Subject: Re: Updating our Code Contribution/Style Guide > I support this too… leads to more noise in, and less readability of, the > patch. Readability of the patch is not harmed with modern tooling (with whitespace being highlighted differently to content changes). Legibility of the code (not patch) should always be preferred IMO. To aid code comprehension, we should aim for density of useful information for the reader; wasting a dozen or more lines on zero information density, solely to solve a problem already handled by modern diff tools, is a false economy. We are talking about one extra line, not a dozen or more. It also improves the readability of the code IMHO. > I would also like to suggest that an operator should always carry on line > wraps For the ternary operator I agree, however I am less convinced in other cases. String concatenation is probably cleaner with the opposite norm, so that string literals are aligned. IMHO it works for string concatenation too. The example that comes to mind is a) method( "a", "b", "c" ) b) method( "a" + "b" + "c" ) c) method( "a" + "b" + "c" ) Once the code gets more real, it is faster to read the difference between (a) and (c) than it (a) and (b).
