1. Use java.util.Optional
instead of null
By using java.util.Optional
you will force clients to check existence of the value.
Consider getBeer(...)
method below, the caller of the method expects to receive a Beer
object and it’s not clear from the method API that the Beer
can be null
. The caller might forget to add a null
check and will potentially receive NullPointerException
which is a programmer error, by the way.
Before
public Beer getBeer(Customer customer) {
return customer.age < 18
? null
: this.beerCatalogue.get(0);
}
After
public Optional<Beer> getBeer(Customer customer) {
return customer.age < 18
? empty()
: Optional.of(this.beerCatalogue.get(0));
}
2. Return empty collection/array instead of null
Motivation is the same as above - we should not rely on null
Before
public List<Beer> getBeerCatalogue(Customer customer) {
return customer.age < 18
? null
: this.beerCatalogue;
}
After
public List<Beer> getBeerCatalogue(Customer customer) {
return customer.age < 18
? emptyList()
: this.beerCatalogue;
}
3. Use var
Type inference reduces the amount of boilerplate code and reduces cognitive complexity, which leads to better readability.
Before
static FieldMapper fieldMapper(FieldDescriptor fieldDescriptor,
SchemaDefinition schemaDefinition) {
ValueGetter valueGetter = valueGetter(schemaDefinition, fieldDescriptor);
return (dynamicMessageBuilder, row) -> {
Iterable<DynamicMessage> iterable = (Iterable<DynamicMessage>) valueGetter.get(row);
for (Object entry : iterable) {
dynamicMessageBuilder.addRepeatedField(fieldDescriptor, entry);
}
};
}
After
static FieldMapper fieldMapper(FieldDescriptor fieldDescriptor,
SchemaDefinition schemaDefinition) {
var valueGetter = valueGetter(schemaDefinition, fieldDescriptor);
return (dynamicMessageBuilder, row) -> {
var iterable = (Iterable<DynamicMessage>) valueGetter.get(row);
for (var entry : iterable) {
dynamicMessageBuilder.addRepeatedField(fieldDescriptor, entry);
}
};
}
4. Make local variables final
Making local variables final
hints to a programmer that the variable can’t be reassigned, which generally leads to better code quality and helps avoid bugs.
Before
static FieldMapper fieldMapper(FieldDescriptor fieldDescriptor,
SchemaDefinition schemaDefinition) {
var valueGetter = valueGetter(schemaDefinition, fieldDescriptor);
return (dynamicMessageBuilder, row) -> {
var iterable = (Iterable<DynamicMessage>) valueGetter.get(row);
for (var entry : iterable) {
dynamicMessageBuilder.addRepeatedField(fieldDescriptor, entry);
}
};
}
After
static FieldMapper fieldMapper(FieldDescriptor fieldDescriptor,
SchemaDefinition schemaDefinition) {
final var valueGetter = valueGetter(schemaDefinition, fieldDescriptor);
return (dynamicMessageBuilder, row) -> {
final var iterable = (Iterable<DynamicMessage>) valueGetter.get(row);
for (final var entry : iterable) {
dynamicMessageBuilder.addRepeatedField(fieldDescriptor, entry);
}
};
}
5. Use static imports
Static imports make code less verbose and hence more readable.
Please note, there is one edge case to this rule - There are a bunch of static methods in Java (List.of()
, Set.of()
, Map.of()
etc.) static importing which would harm code quality, making it ambiguous. So, using this rule, always ask yourself - Does this static import make code more readable?
Before
public static List<FieldGetter> fieldGetters(SchemaDefinition schemaDefinition,
FieldName fieldName,
FieldDescriptor fieldDescriptor) {
final var schemaField = SchemaUtils.findFieldByName(schemaDefinition, fieldName)
.orElseThrow(SchemaFieldNotFoundException::new);
if (fieldDescriptor.getJavaType() == FieldDescriptor.JavaType.MESSAGE) {
return schemaField.getFields().stream()
.flatMap(it -> fieldGetters(schemaDefinition, it.getName(), it.getDescriptor()))
.collect(Collectors.toList());
}
return Collections.emptyList();
}
After
public static List<FieldGetter> fieldGetters(SchemaDefinition schemaDefinition,
FieldName fieldName,
FieldDescriptor fieldDescriptor) {
final var schemaField = findFieldByName(schemaDefinition, fieldName)
.orElseThrow(SchemaFieldNotFoundException::new);
if (fieldDescriptor.getJavaType() == MESSAGE) {
return schemaField.getFields().stream()
.flatMap(it -> fieldGetters(schemaDefinition, it.getName(), it.getDescriptor()))
.collect(toList());
}
return emptyList();
}
6. Prefer fully qualified imports
The same as above, it makes code more readable.
Before
public SchemaDefinition.GraphView makeSchemaDefinitionGraph() {
final var rootNode = new SchemaDefinition.RootNode();
final var messageNode1 = new SchemaDefinition.MessageNode.MessageNodeBuilder()
.withHeader("message-header-1")
.withBody("message-body-1")
.build();
final var messageNode2 = new SchemaDefinition.MessageNode.MessageNodeBuilder()
.withHeader("message-header-2")
.withBody("message-body-2")
.build();
rootNode.addNode(messageNode1);
rootNode.addNode(messageNode2);
return rootNode.asGraph();
}
After
public GraphView makeSchemaDefinitionGraph() {
final var rootNode = new RootNode();
final var messageNode1 = new MessageNodeBuilder()
.withHeader("message-header-1")
.withBody("message-body-1")
.build();
final var messageNode2 = new MessageNodeBuilder()
.withHeader("message-header-2")
.withBody("message-body-2")
.build();
rootNode.addNode(messageNode1);
rootNode.addNode(messageNode2);
return rootNode.asGraph();
}
7. Put each parameter on a new line in long method/constructor declarations
Having a particular code style and using it across the codebase reduces cognitive complexity, meaning that code is easier to read and understand.
Before
public void processUserData(String name, int age, String address, double salary, boolean isEmployed, String occupation) {
//...
}
// or
public void processUserData(
String name, int age, String address, double salary, boolean isEmployed, String occupation
) {
//...
}
// or
public void processUserData(String name, int age,
String address, double salary, boolean isEmployed, String occupation
) {
//...
}
// or
public void processUserData(String name, int age, String address, double salary,
boolean isEmployed, String occupation
) {
//...
}
After
public void processUserData(String name,
int age,
String address,
double salary,
boolean isEmployed,
String occupation) {
//...
}
// or
public void processUserData(
String name,
int age,
String address,
double salary,
boolean isEmployed,
String occupation) {
//...
}
8. Create immutable POJOs or use record
Immutable classes are easier to design, implement, and use than mutable ones. They are less prone to error and are more secure. With immutable objects, you don't have to worry about synchronisation or object state (Was the object initialised or not?).
To make class immutable:
- Make all fields in the class
final
- Create a
constructor
/builder
that will initialize all fields - If the value is optional (can be
null
) usejava.util.Optional
- Use immutable collections or return immutable views in getters (
Collections.unmodifiableList(...)
etc) - Don’t expose methods that modify object’s state
Before
public class User {
private String name;
private int age;
private String address;
private List<Claim> claims;
public String getName() {
return name;
}
public void setName(String name) {
this.name = name;
}
public int getAge() {
return age;
}
public void setAge(int age) {
this.age = age;
}
public String getAddress() {
return address;
}
public void setAddress(String address) {
this.address = address;
}
public List<Claim> getClaims() {
return claims;
}
public void setClaims(List<Claim> claims) {
this.claims = claims;
}
// ...
}
After
public class User {
private final String name;
private final int age;
private final String address;
private final List<Claim> claims;
public User(String name, int age, String address, List<Claim> claims) {
this.name = requireNonNull(name);
this.age = requirePositive(age);
this.address = requireNonNull(address);
this.claims = List.copyOf(requireNonNull(claims));
}
public String getName() {
return name;
}
public int getAge() {
return age;
}
public String getAddress() {
return address;
}
public List<Claim> getClaims() {
return claims;
}
// ...
}
Or using record
if you use java 14+
public record User(String name, int age, String address, List<Claim> claims) {
public User {
requireNonNull(name);
requirePositive(age);
requireNonNull(address);
claims = List.copyOf(requireNonNull(claims));
}
}
9. Use Builder pattern for classes with many parameters/optional parameters
The Builder pattern simulates named parameters available in Python/Scala/Kotlin. It makes client code easy to read and write, and it enables you to work with optionals / parameters with default values more fluently.
Before
public class User {
private final UUID id;
private final Instant createdAt;
private final Instant updatedAt;
private final String firstName;
private final String lastName;
private final Email email;
private final Optional<Integer> age;
private final Optional<String> middleName;
private final Optional<Address> address;
public User(UUID id,
Instant createdAt,
Instant updatedAt,
String firstName,
String lastName,
Email email,
Optional<Integer> age,
Optional<String> middleName,
Optional<Address> address) {
this.id = requireNonNull(id);
this.createdAt = requireNonNull(createdAt);
this.updatedAt = requireNonNull(updatedAt);
this.firstName = requireNonNull(firstName);
this.lastName = requireNonNull(lastName);
this.email = requireNonNull(email);
this.age = requireNonNull(age);
this.middleName = requireNonNull(middleName);
this.address = requireNonNull(address);
if (firstName.isBlank()) {
// throw exception
}
// ... validation
}
// ...
}
// And then you would write:
public User createUser() {
final var user = new User(
randomUUID(),
now(),
now().minus(1L, DAYS),
// firstName, lastName and email are String, what if you
// mix up parameters order in constructor?
"first_name",
"last_name",
"user@test.com",
empty(),
Optional.of("middle_name"),
empty()
);
// ...
return user;
}
After
public class User {
private final UUID id;
private final Instant createdAt;
private final Instant updatedAt;
private final String firstName;
private final String lastName;
private final String email;
private final Optional<Integer> age;
private final Optional<String> middleName;
private final Optional<Address> address;
// private constructor
private User(Builder builder) {
this.id = requireNonNull(builder.id);
this.createdAt = requireNonNull(builder.createdAt);
this.updatedAt = requireNonNull(builder.updatedAt);
this.firstName = requireNonNull(builder.firstName);
this.lastName = requireNonNull(builder.lastName);
this.email = requireNonNull(builder.email);
this.age = requireNonNull(builder.age);
this.middleName = requireNonNull(builder.middleName);
this.address = requireNonNull(builder.address);
if (firstName.isBlank()) {
// throw exception
}
// ... validation
}
// ...
public static class Builder {
private UUID id;
private Instant createdAt;
private Instant updatedAt;
private String firstName;
private String lastName;
private String email;
// Optionals are empty by default
private Optional<Integer> age = empty();
private Optional<String> middleName = empty();
private Optional<Address> address = empty();
private Builder() {
}
public static Builder newUser() {
// You can easily add (lazy) default parameters
return new Builder()
.id(randomUUID())
.createdAt(now())
.updatedAt(now());
}
public Builder id(UUID id) {
this.id = id;
return this;
}
public Builder createdAt(Instant createdAt) {
this.createdAt = createdAt;
return this;
}
// ...
public Builder address(Address address) {
this.address = Optional.ofNullable(address);
return this;
}
public User build() {
return new User(this);
}
}
}
// And then:
public User createUser() {
// You end up writing more code in User class but the
// class API becomes more concise
final var user = newUser()
.updatedAt(now().minus(1L, DAYS))
.firstName("first_name")
.lastName("last_name")
.email("user@test.com")
.middleName("middle_name")
.build();
// ...
return user;
}
Top comments (0)