Best practices - 6.1 Funnelback Java style guide, coding conventions and standards

Background

This document defines formatting and style rules for Java in use at Funnelback.

General formatting and naming rules

Indentation

Indent by 4 spaces at a time.

Don’t use tabs or mix tabs and spaces for indentation.

Do this
public void myMethod() {
    if (isTrue()) {
        doSomething();
    }
}
Don’t do this
public void myMethod() {
	if (isTrue()) {
		doSomething();
	}
}
Definition

Indentation improves readability.

Pros
  • Spaces are displayed identically regardless of the IDE.

  • Makes reading DIFFs easier.

Cons
  • Users cannot chose the indent display size in their IDE.

  • Tabs saves on source file size.

Line width

Line width should not exceed 120 characters.

Variable naming

Variables should be written in camelCase, constants in ALL_CAPS.

Method length

Keep methods short (100 lines maximum).

Default implementations naming

Denote default implementations (of an interface or abstract class) with a Default prefix.

Committing formatting changes

Separate formatting commits from actual code change commits.

Error handling

Use exceptions to signal an error

When an error occurs, throw an Exception rather than returning null or a special value like -1.

Do this
String[] data = getData();
if (data == null) {
    throw new IllegalStateException("No data found");
}
return data.length;

Ideally getData() should throw an Exception itself, simplifying the code as we could just use:

return getData().length;

This assumes that everything went all right, but if something is wrong the underlying getData() call will throw an Exception with the actual cause for the failure.

Don’t do this
String[] data = getData();
if (data == null) {
    return -1;
} else {
    return data.length
}
Definition

It helps to distinguish actual errors against special return values. Some methods might return null on purpose to indicate that no data has been found. Throwing an Exception ensures the caller can distinguish between "No data" and an error case.

It simplifies code for callers as they don’t have to worry about special values. It might also help to avoid NullPointerException and to have a proper stacktrace when something goes wrong.

Custom exception types

Use custom exception types when necessary.

Class design

Constructors

Constructors shouldn’t do too much work.

Use a setUp() method if necessary.

Consider using the Factory or Builder pattern if expensive initialisation is needed.

Single responsibility principle

Classes should only do one thing.

Class fields

Use final fields whenever possible.

Inheritance

Use protected or the default scope with reason.

Only use protected when the class is intended to be extended, use private otherwise.

Implementation

Strong typing

Use Strings for Strings only

Consider using enums if you have a fixed set of values.

Input parameters validation

Validate input parameters (on public methods only?)

Investigate the use of @NotNull. See: [NotNull Annotation](http://stackoverflow.com/questions/4963300/which-notnull-java-annotation-should-i-use)

Return values checking

Check return values for NULLs.

Investigate the use of an Option type ?</p>

Magic numbers and strings

Use constants for magic numbers and strings.

Consider using collection.cfg parameters when the value might be configured by the user.

Reading from config files

Always use a default value fall-back when reading from configuration files.

Always use the method that will return a default values if the value doesn’t exist in he configuration file.

Definition

Using such a pattern guarantees that reading from the config file will always return a value, simplifying the code as you don’t have to check for null values, and preventing NullPointerExceptions.

Do this
Config c = new NoOptionsConfig(...);
String secModel = c.value(Keys.FileCopy.SECURITY_MODEL, DefaultValues.FileCopy.SECURITY_MODEL_NONE)
int maxFilesStored = c.valueAsInt(Keys.FileCopy.MAX_FILES_STORED, DefaultValues.FileCopy.MAX_FILES_STORED);
Don’t do this
Config c = new NoOptionsConfig(...);

// No value check, could be null
String secModel = c.value(Keys.FileCopy.SECURITY_MODEL);

// Unnecessary complex check
String secModel = c.value(Keys.FileCopy.SECURITY_MODEL);
if (secModel == null || "".equals(secModel)) {
	secModel = "none";
}
Pros
  • Simplifies code

  • Prevents NullPointerException

Cons
  • Default values in the DefaultValues class might get out of sync with collection.cfg.default.

Collection-types return values

Return copies or immutable version of collections.

This applies only if those collections holds states in your class (i.e. are a field of the class)

Regular expressions

Document regular expressions properly.

Break them down in smaller part if possible. (See: Reporting updating code.)

Use approved libraries

Use the libraries used everywhere else for common tasks.

Such libraries include:

  • Log4j for logging

  • Apache Commons for general purpose utilities

  • Project Lombok

  • Java built-in XML parsers (SAX, DOM, Stream)

Comments

Use Javadoc

Use Javadoc where possible (as opposed to plain comments).

Comments scope

Document all method, classes (file header), constants.

TODO getters / setters ?

Comments content

Ensure comments contain relevant information.

A comment should contain:

  • Context

  • Expectations

  • Results

  • Unexpected behaviors

  • Limitations

Dead code

Remove commented out code.

Commented code is confusing as people reading the source can’t really know if it’s important or not.

There’s no justification for it:

  • If the code is supposed to be removed, just remove it.

  • If history needs to be kept, SVN can be consulted to retrieve a previous version.

  • If the code is supposed to be deprecated, annotate the method but don’t comment it out.

Resource naming

Use a consistent prefix for generated Java resources

Use a consistent prefix like 'funnelback-' for generated artifacts (e.g. .jar and .war files).

Configure Maven to implement this

Ensure the artifactId setting in your module’s pom.xml file starts with the required prefix.

Deprecated approaches and libraries

This section lists approaches, libraries and classes whose use is now deprecated.

They may still be present in existing code but you should not be using them in new work.

Java serialization

Avoid using Java serialization in favor of a custom format or database to serialize data.

Pros
  • Comes for free

Cons
  • Hurts interoperability, serialized data cannot be read outside Java

  • Cause problems during upgrades with different versions of the serialized objects

  • serialVersionUid needs to be manually maintained

ObjectCache

Avoid using the ObjectCache interface for new development.

The ObjectCache interface has various legacy problems that cascaded to its implementations, resulting in some parameters not being used or some method not implemented.

Its current design makes assumptions on the storage implementation, making it hard to implement for different storage systems.

Definition

Avoid using ObjectCache and its implementations: BTreeCache, SQLiteCache, RedisCache, …​

Finally

Be consistent.

If you’re editing code, take a few minutes to look at the code around you and determine its style. If they use spaces around all their arithmetic operators, you should too. If their comments have little boxes of hash marks around them, make your comments have little boxes of hash marks around them too.

The point of having style guidelines is to have a common vocabulary of coding so people can concentrate on what you’re saying rather than on how you’re saying it. We present global style rules here so people know the vocabulary, but local style is also important. If code you add to a file looks drastically different from the existing code around it, it throws readers out of their rhythm when they go to read it. Please try to avoid this.