[cassandra-website] branch trunk updated: Update code_style.adoc

[be...@apache.org]

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 <be...@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-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org