...
Table of Contents maxLevel 3
Indentation and spaces
Four spaces, NOT a tab character, is used for regular indentation. For code acrid broken into multiple lines, eight spaces are used indent lines after the first:
Code Block language java if (obj instanceof DefaultDevice) { return Objects.equals(this.id, other.id) && Objects.equals(this.type, other.type) && Objects.equals(this.manufacturer, other.manufacturer);
If the line has to break at the dot separator ".", the dot symbol should be at the beginning of the next line:
Code Block language java public void describeTo(Description description) { description.appendText("status '") .appendText(status.getDescription()) .appendText("'"); }
- Trailing whitespaces should be removed in both code and comments.
There should be a space between keywords (
if
,for
,catch
,
etc.), parenthesis, and braces, as well as when casting:Code Block language java if (timer != null) { try { DeviceId did = (DeviceId) elementId; ... } catch (IOException e) { ...
There should be a space before the curly braces in method definitions. Arguments are not padded (neither in definition nor invocation):
Code Block language java public Device getDevice(DeviceId deviceId) { return store.getDevice(deviceId); }
The project uses Checkstyle to enforce some of the formatting mentioned above. Violations will prevent the build from succeeding.
Comments
Javadocs are used to document the codebase itself. Interfaces are heavily documented with Javadocs so that implementing methods (annotated with @Override
) inherit the commenting.
...
Code Block | ||
---|---|---|
| ||
/** * Implementing classes should have Javadocs as well, to emphasize * their function. */ public class fooImpl implements foo { /** * Important variables and structures should also be * commented so that it is picked up by Javadocs. */ private static final int SAMPLE = 5; @Override public boolean sampleMethod(int param) { // classic one-liner boolean val = false; /* * Multi-line comments within the code may use this * convention. */ if (param < SAMPLE) { // FIXME: multiple lines may be commented like this // for code that may be removed or changed // return true; val = true; } return val; } } |
Interfaces and classes
Naming
...
Interfaces should always be given first pick of clear names that convey their purpose. For example, if there is an interface representing a network device, and a class implementing it, the interface should be given the name Device
, and the class, something indicating that it implements the Device
interface, e.g. DefaultDevice
.
Referencing
...
Wherever possible, references should be made to the interface, and not the implementing class. This includes method parameters.
Nested (Inner) classes
...
A class that implements functions specific to a particular class (e.g. its event handlers or services that it exports) should be implemented as an inner private class within the class. Such classes have names beginning with Internal-
, e,g InternalClusterEventListener
.
Objects and Methods
Data types
...
Wherever possible, use a rich data type over a primitive (e.g. MACAddress
versus a long
). This reduces ambiguity.
Immutable objects
- Classes that .
Class variables should be declaredfinal
for classes that are intended to be instantiated as immutable objects (e.g.DefaultDevice
) should have class all variables declaredfinal
. Collections (
Sets
,Maps
, etc) returned by getters should be immutable, or a copy of the existing Set.Code Block language java @Override public Iterable<Device> getDevices() { return Collections.unmodifiableCollection(devices.values()); } // or, if copying @Override public Set<DeviceId> getDevices(NodeId nodeId) { Set<DeviceId> ids = new HashSet<>(); for (Map.Entry<DeviceId, NodeId> d : masterMap.entrySet()) { if (d.getValue().equals(nodeId)) { ids.add(d.getKey()); } } return ids; }
Concurrency
- Avoid
synchronized(this)
and synchronized methods wherever possible. Opt for thread-safe objects such as
ConcurrentMap
, and if usingsynchronized
, apply it to the structure that must be locked:Code Block language java protected final Map<DeviceId, LinkDiscovery> discoverers = new HashMap<>(); @Override public void event(MastershipEvent event) { ... synchronized (discoverers) { if (!discoverers.containsKey(deviceId)) { discoverers.put(deviceId, new LinkDiscovery(device, packetSevice, masterService, providerService, useBDDP)); } } }
Additionally, some structures such as Hazelcast's
IMap
have per-key locks.
Logging
The codebase uses SLF4J for logging. DO NOT use System.out.*
.
The logger should be private final, and associated with the particular class:
Code Block | ||
---|---|---|
| ||
private final Logger log = getLogger(getClass());
...
/**
* Checks if all the reachable devices have a valid mastership role.
*/
private void mastershipCheck() {
log.debug("Checking mastership");
for (Device device : getDevices()) { |
When adding logs for debugging, rather than using info
or warn
, use the debug
or trace
verbosity level and set the log level in Karaf to inspect the debug messages.
Usage of Guava (More of a tools section topic)
ONOS leverages guava in several areas. Some prominent areas are:
Checking for null method inputs - using checkNotNull():
Code Block language java @Override public void removeDevice(DeviceId deviceId) { checkNotNull(deviceId, DEVICE_ID_NULL); DeviceEvent event = store.removeDevice(deviceId); if (event != null) { log.info("Device {} administratively removed", deviceId); post(event); } }
toString() - ToStringHelper():
Code Block language java @Override public String toString() { return toStringHelper(this) .add("id", id) .add("vlan", vlan) .add("ipAddresses", ips) .toString(); }
- Data structures such as
Lists
,ImmutableSet
, andHashMultiMap
Unit testing - EqualsTester():
Code Block language java VlanId vlan1 = VlanId.vlanId((short) -1); VlanId vlan2 = VlanId.vlanId((short) 100); VlanId vlan3 = VlanId.vlanId((short) 100); // first two equality groups contain equal objects, last differs // from constituents of first two groups new EqualsTester().addEqualityGroup(VlanId.vlanId(), vlan1) .addEqualityGroup(vlan2, vlan3) .addEqualityGroup(VlanId.vlanId((short) 10));