Virgo/Committers/Coding

Code Style

All Virgo code must adhere to the following code style requirements.

Code Formatting and Templates

Eclipse code templates, formatter, and clean up profile are available in the packaging git repository in the doc/code-style/eclipse directory. Please use them.

All code committed to the repository must adhere to these templates and must be formatted to match this code style.

A note on the New File template

In the code templates for Eclipse, there is a copyright and licence header for new Java files. This will generate a partial copyright statement which must be updated with the name of the copyright holder before the code is committed. For example:

/*
 * This file is part of the Eclipse Virgo project.
 * 
 * Copyright (c) 2010 copyright_holder
 * ...
 */

should be updated to, for My Company Inc.:

/*
 * This file is part of the Eclipse Virgo project.
 * 
 * Copyright (c) 2010 My Company Inc.
 * ...
 */

before committing. The easiest way of achieving this is to update the Comment->File template in the Eclipse Java Code templates after you have imported the templates and before you create new files.

Coding Conventions

Virgo code conforms to the Sun Java Coding Conventions except for the specific deviations from these conventions detailed below.

Interface definitions

Interface method definitions must not include any visibility modifier or the 'abstract' modifier. The code below shows an incorrect interface definition:

public interface MyInterface {

	public abstract void doSomething();
}

The correct definition is shown below:

public interface MyInterface {

	void doSomething();
}

Final parameters and local variables

The use of the final qualifier on method parameters and local variables is strongly discouraged. The reason is that these uses of final have no actual benefit (or semantic effect) in practice (even as a hint to the compiler) and that the keyword simply makes the code more verbose -- and therefore harder to read.

Valid scenarios for final usage are where the keyword does have semantic effect, including:

• passing state to anonymous type instances; and
• preservation of an important internal method contract.

Typically, instance variables (state) in instantiated classes will be declared final whenever it is possible to do so as this is a significant contributor to thread-safe code.

(Note that this can lead to large numbers of parameters on constructors of complex classes. See the Builder pattern in Effective Java for ways round this.)

Concurrent Programming Guidelines

Levels of Concurrent Support

A class can either be thread safe or not. A class is thread safe if its invariants are protected when accessed simultaneously by multiple threads.

Required Level of Concurrent Support

• All classes that form part of the public API of their bundle, or serve as an externally accessible implementation of that public API are required to be thread safe.

• Public classes that are not intended to be shared by multiple threads may be coded as non thread safe, but this in discouraged since single-threaded usage cannot easily be enforced.

• Classes that are internal to a bundle are free to be non thread safe. However, if these classes are used as part of the implementation of the bundle API then the API classes must guarantee thread safe access to their delegate classes.

Alien Calls

An alien call is any call to a component outside of the control of the caller. Holding a lock while making an alien call is deadlock-prone and forbidden. The big question is what constitutes a component. Treating each class as a component from the perspective of alien calls is safe but can be too limiting at times. A package (within a bundle) is a more useful component in practice. In some situations, it may be necessary to code alien calls across packages within a bundle, but extreme care must be taken to avoid deadlocks. Alien calls between bundles are forbidden.

Use of Client Locking Protocols

A client locking protocol is a contract between a class and its consumers. This contract outlines how the calling class can cooperate in the locking of the callee class. An example of a client locking protocol in common usage can be found in Hashtable. When performing composite operations on a Hashtable it is possible to synchronize on the Hashtable instance and participate cooperatively in the locking inside the Hashtable.

In Virgo, the use of client locking protocols is forbidden because they can very easily lead to deadlocks. If a class in Virgo needs to allow composite operations on its internal data structures, it should support these operations in its API.

Use of synchronized Methods

The use of synchronized methods in Virgo is forbidden. A class with synchronized methods implicitly exposes its monitor object (itself) to the outside world.

In a multi-threaded environment this can easily lead to deadlocks. If class Foo synchronizes on an instance of Bar, and these instances of Bar synchronize on 'this', then access of Foo and Bar across different threads can easily deadlock.

Note also that explicit 'synchronized(this)' blocks are also forbidden.

A simple alternative for classes wishing to maintain some internal synchronization is to use an internal monitor field:

public class MyClass {

	private final Object monitor = new Object();
	
	public void doSomething() {
		synchronized(this.monitor) {
			// logic here
		}
	}
}

Important: The monitor field must be declared as final otherwise synchronizing on it is non-deterministic due to Java Memory Model visibility rules.

Code Visibility

Types

Types should always have the minimum visibility consistent with their use.

Methods

Methods should have the minimum visibility consistent with their use. Methods that form part of the public contract of a type should of course be public but methods that are used to implement that contract should be private.

When allowing extension, protected methods can be used to provide extension points for subclasses. Protected final methods may be used to provide functionality to subtypes but that cannot be extended.

It is valid to relax the visibility of methods to make unit testing easier. In this case, it is common to change a private method to have default access so that test classes in the same package can access it.

Static Fields

For immutable static fields, such as those used in constants, any access modifier is valid. For mutable static state, private access must always be used and mutations to the state should be done via accessor methods.

Instance Fields

In almost all cases fields should be private. Providing subclasses with access to state can be done with protected fields but protected accessor methods are preferred.

Type Naming Conventions

The following type naming conventions must be adhered to for both runtime and test classes.

Interfaces

When implementing an interface the use of the 'Impl' suffix is not permitted.

Ideally choose a name that truly reflects the implementation detail. An example of this is XmlConfigurationSource, an implementation of ConfigurationSource that uses XML to read configuration data.

If no better name is available, the implementation should be named by prefixing Standard to the interface name. For example, the interface Kernel could have an implementation named StandardKernel.

Test Cases

All test case types must have their names suffixed with 'Tests', for example StandardKernelTests.

JavaDoc Guidelines

Useful Resources

• Sun Writing Doc Comments Guide
• JavaDoc Tag Summary

Documentation Requirements

API documentation must be written to conform to the guidelines set out in Sun's Requirements for Writing API Specifications. This document is full of useful examples from the JDK.

In addition to the requirements from this document all types, and methods where relevant, must document their concurrent behavior. Types should document their concurrent semantics including whether or not they are thread safe.

Documenting Interface Concurrency

Interfaces must document any restrictions that their contract places on the concurrency of an implementation. For example, an interface may require that a Collection returned from one of its methods must reflect internal updates accurately or must throw ConcurrentModificationException if a change occurs during iteration. For example:

/**
 * ...
 *
 * <strong>Concurrent Semantics</strong><br/>
 * 
 * Implementations must be thread safe.
 */
public interface ...

Documenting Type-Level Concurrency

Types must as a minimum document whether or not they are thread safe. In addition, types must document any lock acquisition protocols that they require calling code to adhere to.

Also, types must document the guarantees around state visibility that they provide as part of their implemented concurrency level. For example, some types may present internal Collection state in a weakly consistent manner.

Example:

/**
 * ...
 * 
 * <strong>Concurrent Semantics</strong><br/>
 * 
 * Implementation is immutable and therefore thread safe.
 * 
 */

Documenting Method-Level Concurrency

For methods that have specific concurrency semantics, these semantics should be included in the JavaDoc. For example:

/**
 * @inheritDoc
 * <p/>
 * Returns a weakly consistent view of the <code>Subsystems</code>.
 * This is guaranteed to be consistent with the view of <code>Subsystems</code>
 * when this method is called. Concurrent modifications are not guaranteed to be
 *  reflected.
 * @see ConcurrentHashMap#values()
 * 
 */
public Collection<Subsystem> getSubsystems() {
    ...
}