This is an automated email from the ASF dual-hosted git repository.
benedict pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra-website.git
The following commit(s) were added to refs/heads/trunk by this push:
new 624d5e73f Update code_style.adoc
624d5e73f is described below
commit 624d5e73fe15406777f0f45f7799bcf096c46088
Author: Benedict Elliott Smith <bened...@apache.org>
AuthorDate: Tue Jun 7 21:33:58 2022 +0100
Update code_style.adoc
---
.../modules/ROOT/pages/development/code_style.adoc | 193 ++++++++++++++-------
1 file changed, 129 insertions(+), 64 deletions(-)
diff --git a/site-content/source/modules/ROOT/pages/development/code_style.adoc
b/site-content/source/modules/ROOT/pages/development/code_style.adoc
index 99af33171..3e18e8ebd 100644
--- a/site-content/source/modules/ROOT/pages/development/code_style.adoc
+++ b/site-content/source/modules/ROOT/pages/development/code_style.adoc
@@ -1,85 +1,150 @@
= Code Style
:page-layout: basic
-== Code Style
+The Cassandra project follows
+http://java.sun.com/docs/codeconv/html/CodeConvTOC.doc.html[Sun's Java
+coding conventions] for anything not expressly outlined in this document.
-=== General Code Conventions
+Note that the project has a variety of styles that have accumulated in
different subsystems. Where possible a balance should be struck between these
guidelines and the style of the code that is being modified as part of a patch.
Patches should also limit their scope to the minimum necessary for safely
addressing the concerns of the patch.
-* The Cassandra project follows
-http://java.sun.com/docs/codeconv/html/CodeConvTOC.doc.html[Sun's Java
-coding conventions] with one important exception: `{` and `}` are always
-placed on a new line.
-
-=== Exception handling
-
-* Never ever write `catch (...) {}` or `catch (...) { logger.error() }`
-merely to satisfy Java's compile-time exception checking. Always
-propagate the exception up or throw `RuntimeException` (or, if it "can't
-happen," `AssertionError`). This makes the exceptions visible to
-automated tests.
-* Avoid propagating up checked exceptions that no caller handles.
-Rethrow as `RuntimeException` (or `IOError`, if that is more
-applicable).
-* Similarly, logger.warn() is often a cop-out: is this an error or not?
-If it is don't hide it behind a warn; if it isn't, no need for the
-warning.
-* If you genuinely know an exception indicates an expected condition,
-it's okay to ignore it BUT this must be explicitly explained in a
-comment.
-
-=== Boilerplate
-
-* Avoid redundant `@Override` annotations when implementing abstract or
-interface methods.
-* Do not implement equals or hashcode methods unless they are actually
-needed.
-* Prefer public final fields to private fields with getters. (But prefer
-encapsulating behavior in "real" methods to either.)
-* Prefer requiring initialization in the constructor to setters.
-* Avoid redundant `this` references to member fields or methods.
-* Do not extract interfaces (or abstract classes) unless you actually
-need multiple implementations of it.
-* Always include braces for nested levels of conditionals and loops.
-Only avoid braces for single level.
-
-=== Multiline statements
-
-* Try to keep lines under 120 characters, but use good judgement.
-It is better to exceed 120 by a little, than split a line that has no natural
-splitting points.
-* When splitting inside a method call, use one line per parameter and
-align the items called:
-
-[source,none]
+== Naming and Clear Semantics
+
+==== Class, Method and Variable Naming
+
+Avoid extraneous words, for example prefer `x()` over `getX()` or `setX()`
where it makes semantic sense. At the same time, do not avoid using words that
are necessary, for example if a descriptive word provides semantic context such
as `liveReplicas` over `replicas`. This is essential when there are many
conceptual instantiations for a variable that are not enforced by the type
system, but be sure to be consistent in the word choice and order across all
instantiations of the variable.
+
+e.g. _allReplicas_, _naturalReplicas_, _pendingReplicas_, _allLiveReplicas_,
etc.
+
+==== Method and Variable Naming Consistency
+Ensure consistency of naming within a method, and between methods. It may be
that multiple names are appropriate for a concept, but these should not be
mixed and matched within the project. If you modify a concept, or improve the
naming of a concept, make all relevant - including existing - code consistent
with the new terminology. If possible, correspond with a prior author before
modifying their semantics.
+
+===== Standard word meanings in method or property names
+[cols="1,1"]
+|===
+|`calculateX`,`computeX`|Perform some potentially expensive work to produce x
+|`refreshX`,`updateX`|Recompute a memoized x
+|`lookupX`|Find x in a map, or other structure, that is efficient but not free
+|`x`|Return x, relatively cheaply
+|`toX`|Return a potentially expensive translation to x
+|`asX`|Return a cheap translation to x
+|`asXView`|Return a cheap translation to x, that will reflect changes in the
source
+|`isX`, `hasX`, `canX`|Boolean property or method indicating a capability or
logical state
+|===
+
+For boolean variables, fields and methods, choose names that sound like
predicates and cannot be confused with nouns.
+
+==== Semantic Distinctions via the Type System
+
+If possible, enforce semantic distinctions at compile time with the type
system.
+
+_e.g. `RangesAtEndpoint`, `EndpointsForRange` and `EndpointsForToken` are all
semantically different variants on a collection of replicas._
+
+This makes the intent of the code clearer, and helps the compiler indicate
where we may have unintentionally conflated concepts. They also provide
opportunities to insert stronger runtime checks that our assumptions hold, and
these constraints can provide further clarity when reading the code.
+
+_In the case of `EndpointsForX`, for instance, we enforce that we have no
duplicate endpoints, and that all of the endpoints do fully cover X._
+
+===== Enums for Boolean Properties
+Prefer an `enum` to `boolean` properties and parameters, unless clarity will
be harmed (e.g. helper methods that accept a computed boolean predicate result,
of the same name as used in the method they assist). Try to balance name
clashes that would affect static imports, against clear and simple names that
represent the behavioural switch.
+
+==== Semantic Distinctions via Member Variables
+If a separate type for all concepts is too burdensome, a type that aggregates
concepts together within member variables might be applicable.
+
+The most obvious counter-example is not to use `Pair`, or a similar tuple.
Unless it is extremely obvious, prefer a dedicated type with well named member
variables.
+
+_For example, `FetchReplicas` for source and target replicas, and
`ReplicaLayout` for the distinction between natural and pending replicas._
+
+This may help authors notice other semantics they had overlooked, that might
have led to subtly incorrect parameter provision to methods. Conversely,
methods may choose to accept one of these encapsulating types, so that callers
do not need to consider which member they should provide.
+
+_e.g. `ConsistencyLevel.assureSufficientLiveReplicas` requires very specific
replica collections, that are quite distinct, that might be easily incorrectly
provided (though this is still inadequate, as it needs to distinguish between
live and non-live semantics, which remains to be improved)_
+
+==== Public APIs
+These considerations are especially important for public APIs, including CQL,
virtual tables, JMX, yaml, system properties, etc. Any planned additions must
be carefully considered in the context of any existing APIs. Where possible the
approach of any existing API should be followed. Where the existing API is
poorly suited, a strategy should be developed to modify or replace the existing
API with one that is more coherent in light of the changes - which should also
carefully consider any [...]
+
+== Code Structure
+==== Necessity
+If an interface has only one implementation, remove it. If a method isn’t
used, delete it.
+
+Don’t implement `hashCode()`, `equals()`, `toString()` or other methods unless
they provide immediate utility.
+
+==== Specificity
+Don’t overgeneralise. Implement the most specific method or class that you
can, that handles the present use cases.
+
+Methods and classes should have a single clear purpose, and should avoid
special-cases where practical.
+
+==== Class Layout
+Consider where your methods and inner classes live with respect to each other.
Methods that are of a similar category should be adjacent, as should methods
that are primarily dependent on each other. Try to use a consistent pattern,
e.g. helper methods may occur either before or after the method that uses them,
but not both; method signatures that cover different combinations of parameters
should occur in a consistent order visiting the parameter space.
+
+Class declaration order should, approximately, go: inner classes, static
properties, instance properties, constructors (incl static factory methods),
getters/setters, main functional/API methods, helper (incl static) methods and
classes. Clarity should always come first, however.
+
+==== Method Clarity
+A method should be short. There is no hard size limit, but a filled screen is
a good warning size. However, be careful not to over-minimise your methods; a
page of tiny functions is also hard to read.
+
+The body of a method should be limited to the main conceptual work being done.
Substantive ancillary logic, such as computing an intermediate result,
evaluating complex predicates, performing auditing, logging, etc, are prime
candidates for helper methods.
+
+==== Compiler Assistance
+Always use `@Override` annotations when implementing abstract or interface
methods or overriding a parent method.
+
+`@Nullable`, `@NonNull`, `@ThreadSafe`, `@NotThreadSafe` and `@Immutable`
should be used as appropriate to communicate to both the compiler and readers.
+
+==== Boilerplate
+Prefer `public final` fields to private fields with getters (but prefer
encapsulating behavior in "real" methods to either).
+
+Declare class properties `final` wherever possible, but never declare local
variables and parameters `final`. Variables and parameters should still be
treated as immutable wherever possible, with explicit code blocks introduced as
necessary to minimize the scope of any mutable variables.
+
+Prefer initialization in a constructor to setters, and builders where the
constructor is complex with many optional parameters.
+
+Avoid redundant `this` references to member fields or methods, except for
consistency with other assignments e.g. in the constructor
+
+==== Exception handling
+Never ever write `catch (…)` {} or `catch (…) { logger.error() }` merely to
satisfy Java’s compile-time exception checking.
+
+Always catch the narrowest exception type possible for achieving your goal. If
Throwable must be caught for handling exceptional termination, it must be
rethrown. If an exception cannot be safely handled locally, propagate it - but
use unchecked exceptions if no caller expects to handle the case. Rethrow as
`RuntimeException`, `IOError`, or your own `UncheckedXException`, or
`IllegalStateException` if it “can’t happen”
+
+Only if an exception is an explicitly acceptable condition can it be ignored,
but this must be explained carefully in a comment detailing how this is handled
correctly.
+
+== Formatting
+`{` and `}` are placed on a new line except when empty or opening a multi-line
lambda expression. Braces may be elided to a depth of one if the condition or
loop guards a single expression.
+
+Lambda expressions accepting a single parameter should elide the braces that
encapsulate the parameter. E.g. `x -> doSomething()` and `(x, y) ->
doSomething()`
+
+==== Multiline statements
+Where possible prefer keeping a logical action to a single line. Prefer
introducing additional variables, or well-named methods encapsulating actions,
to multi-line statements - unless this harms clarity (e.g. in an already short
method).
+
+Try to keep lines under 120 characters, but use good judgment. It is better to
exceed this limit, than to split a line that has no natural splitting points,
particularly when the remainder of the line is boilerplate or easily inferred
by the reader.
+
+If a line wraps inside a method call, first extract any long parameter
expressions to local variables before trying to group natural parameters
together on a single line, aligning the start of parameters on each line, e.g.
+
+[source,java]
----
-SSTableWriter writer = new SSTableWriter(cfs.getTempSSTablePath(),
- columnFamilies.size(),
- StorageService.getPartitioner());
+Type newType = new Type(someValueWithLongName,
someOtherRelatedValueWithLongName,
+ someUnrelatedValueWithLongName,
+ someDoublyUnrelatedValueWithLongName);
----
-* When splitting a ternary, use one line per clause, carry the operator,
-and align by indenting with 4 white spaces:
+When splitting a ternary, use one line per clause, carry the operator, and
where possible align the start of the ternary condition, e.g.
-[source,none]
+[source,java]
----
var = bar == null
- ? doFoo()
- : doBar();
+ ? doFoo()
+ : doBar();
----
-=== Whitespace
+It is usually preferable to carry the operator for multiline expressions, with
the exception of some multiline string literals.
+
+==== Whitespace
+Make sure to use 4 spaces instead of the tab character for all your
indentation.
+Many lines in the current files have a bunch of trailing whitespace. If you
encounter incorrect whitespace, clean up in a separate patch. Current and
future reviewers won’t want to review whitespace diffs.
-* Make sure to use 4 spaces instead of the tab character for all
-your indentation.
-* Many lines in the current files have a bunch of trailing whitespace.
-If you encounter incorrect whitespace, clean up in a separate patch.
-Current and future reviewers won't want to review whitespace diffs.
+==== Static Imports
+Consider using static imports for frequently used utility methods that are
unambiguous. E.g. `String.format`, `ByteBufferUtil.bytes`,
`Iterables.filter/any/transform`.
-=== Imports
+When naming static methods, select names that maintain semantic legibility
when statically imported, and are unlikely to clash with other method names
that may be mixed in the same context.
+==== Imports
Observe the following order for your imports:
-[source,none]
+[source,java]
----
java
[blank line]
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
