- All Terra Java code should adhere to the Google Java Style Guide where possible.
- All repositories should be set up with auto-formatting and static analysis.
- For stylistic concerns that cannot be automatically enforced, see the guidelines below.
None yet.
We believe developers should strive to spend as little time as possible worrying about and discussing code style. Code formatting and static analysis programs can be helpful tools in pursuit of that goal, since automatic enforcement at compile-time relieves the team from discussing such issues in PR reviews.
All Terra repos should use the following tools:
- Spotless Gradle plugin for auto-formatting. (See example. Devs may also find it useful to set up google-java-format for IntelliJ.)
- Gradle Quality plugin for static analysis with Spotbugs, PMD, and Checkstyle.
We will continually evaluate additional static analysis tools (Error Prone and Codacy) for inclusion in the above list. We should err towards adopting any tool that can provide low-friction enforcement of the key conventions or best practices below.
The sections below summarize our teams’ collective wisdom regarding various Java coding topics. Some advice is meant to be stronger than others — look out for the following keywords:
- always – always follow this practice.
- should – strong guidance; should be followed with few exceptions.
- prefer – softer guidance, with numerous exceptions.
- may / optional / at your discretion – indicates a matter of taste or personal style.
To learn more about Java best practices, see Effective Java, 3rd edition. Many of the guidelines below are paraphrasing Effective Java chapters; look for explicit references to chapters from the book, with links to the https://github.com/david-sauvage/effective-java-summary notes repo.
Unless there is a meaningful non-alphabetical order, prefer alphabetized variable lists, enum entries, etc. in order to make lists easier to visually scan. (And in cases where there is some meaningful alternate ordering, add an explicit comment so future editors can maintain that order.)
For example, these subheadings are alphabetized for lack of a meaningful non-alphabetic order.
We adopt a few general guidelines:
- Minimize accessibility of classes and members.
- Favor composition over inheritance.
- Design and document for inheritance or else prohibit it.
- Prefer interfaces over abstract classes.
See EJ3 #15-25 for more tips.
Code that uses Closeable resources (such as input streams or sockets) should use the try-with-resources pattern:
static String readFirstLineFromFile(String path) throws IOException {
try (BufferedReader br =
new BufferedReader(new FileReader(path))) {
return br.readLine();
}
}
Note that some Streams, such as those returned from the java.nio.file.Files API, may be bound to system resources and must be explicitly closed (docs). Take care to use the try-with-resources pattern in those cases.
Comments, as distinguished from Javadoc (see Javadoc section below), should be used to inform the reader when code cannot be self-explanatory.
A few scenarios deserving of a code comment:
- When it's not obvious why a given line of code exists.
- If a question or request for clarification might come up in code review (or if such a question did come up in code review).
- If a future maintainer might reasonably undo this change or inadvertently break something non-obvious.
- Where the structure or reasoning behind the code is well-explained by an external resource (e.g. StackOverflow link, Jira ticket, or design doc).
For Spring-aware code (e.g. most service and terra-common-lib code), dependencies should be @Autowired via the class constructor. Manual instantiation of @Component classes should be rare.
Exception: Stairway is not Spring-aware, so it is a common pattern to create a dependency-injected class that wraps Stairway functionality.
Checked exceptions place a burden on the users of an API. They are a tool best used when both of the following are true: (1) the exception cannot be prevented by proper use of the API, and (2) the user can take some meaningful action when encountering the exception. Unless both conditions are met, checked exceptions should be avoided. (See EJ3 #71 for more discussion.)
Guidance for unchecked exceptions:
- More specific exceptions should be thrown instead of generic RuntimeExceptions.
- For example: IllegalArgumentException, ConcurrentModificationException, UnsupportedOperationException are commonly reused exceptions from the Java library.
- Other Terra-specific runtime exceptions should be centralized in terra-common-lib.
- Service repos may also house application-specific exceptions (WSM example).
- In many cases, exceptions are explicitly associated with an HTTP status code (example) and used to auto-generate an appropriate response (example).
- Classes – unless a class has been designed and documented for inheritance, it should be marked as final.
- Function parameters – prefer not marking as final, due to the noisiness of the “final” keyword. Use where necessary for occasional semantic emphasis.
- Instance variables – always mark as final if it is never reassigned.
- Local variables – use of final is optional. Short and simple methods can help avoid inappropriate variable reassignment.
- Static variables – almost always mark as final.
Quoting from Effective Java: "immutable classes are easier to design, implement, and use than mutable classes. They are less prone to error and are more secure." See recommendations from Effective Java on how to minimize mutability.
Developers may consider using a library for auto-generating immutable data transfer objects (DTOs). Autovalue is actively used within Workspace Manager and Resource Buffer. Immutables is another popular option.
For constructing immutable collections, prefer Guava immutable collections.
Use Java 8 features where helpful. See https://leanpub.com/whatsnewinjava8/read and https://www.oracle.com/java/technologies/javase/8-whats-new.html for more details.
Here is a quick run-down of Java 8 topics:
- *java.time. – **the JavaTime library should be used for date-time manipulation. Duration is especially common. For non-core extensions, consider the https://www.threeten.org/threeten-extra/ library.
- Lambda expressions – prefer lambda expressions for short and simple bits of code. If a lambda gets too complex, extract it into a named method.
- Method references – prefer method references where possible (e.g.
someStream().map(MyClass::getBar)). - Streams – prefer using Streams for operations that can be framed as a simple, compact stream expression. On the other hand: streams can interfere with exception handling, and complex streams may be more difficult to reason about than equivalent non-stream code, so judgement is required.
Javadoc comments (/** … */) are meant to explain the intended purpose of the element they are attached to. See the style guide for a canonical list of when Javadoc is required.
Some guidelines for writing good Javadoc:
- Spend more time documenting classes that will be frequently used or updated. Documentation mitigates against the risk of future misinterpretation.
- Block tags (
@paramand@return) are generally considered optional:- Use
@paramblock tags when detail is required to clearly convey the meaning of each function parameter. - Use a
@returnblock tag if the main text cannot clearly express the meaning of the returned value in a natural way.
- Use
Some guidelines around Java methods:
-
Keep methods short and simple.
-
Avoid unwieldy parameter lists.
-
Beware of accepting or returning null.
-
Prefer specifying more general types for method parameters (e.g.
List) and more specific types for return values (e.g.ImmutableList). For return types, think about the most specific type which (1) you are comfortable committing to and (2) captures important semantics (i.e. immutability). -
Avoid output parameters (i.e. parameters whose main purpose is to store function output). Prefer a method signature like
Answer foo(args...)instead of
void foo(args..., Answer mutableAnswer)
See EJ3 #49-56 for more tips.
Avoid null wherever possible! Of course, this isn’t always possible, but null checks and @Nullable annotations should always be viewed with healthy skepticism.
@Nullable may be used to indicate parameters or return values where null values are expected. When used, this annotation should be in addition to (but not instead of!) Javadoc comments indicating where and when null values are expected.
@NotNull and @Nonnull can be noisy; they should not be used, unless there is a particular reason that readers might otherwise expect an argument to be nullable.
Optional is... optional :)
java.util.Optional is often a good alternative to a null reference, especially as a method return type (ref). It is not a cure-all, however, and its use is neither required nor disallowed in Terra.
Guidelines:
- Optional should not be used as a function parameter (see this post for some discussion on the topicref).
- Usage within function bodies and as a return value is a matter of preference.
- Optional usage patterns should be consistent within a class (and ideally throughout a service repo).
A class should expose only the minimum-required set of methods and classes for use by clients. This is especially critical for common library developers, but service-level code also benefits from package-level visibility control.
Always use the @VisibleForTesting annotation on methods which are made public or package private just for testing purposes.