Page History

...

Exceeding the maximum line-length of 120 characters will fail the build.

Comments 

Javadoc Comments

Javadocs are used to document the APIs of the codebase itself. Interfaces are heavily documented with Javadocs so that implementing methods (annotated with @Override) inherit the commenting.

Clarity of comments is important and therefore it is important to pay attention to detail and try to write concise, yet meaningful and clear comments.

Here are a few specific number of Javadoc conventions that come up frequently need to be addressed in reviews frequently:

...

...

•  the use of <p> (but not <p></p> or <p/>) should be used to mark paragraph breaks
• <pre> ... </pre> can be used for blocks of monospaced text (e.g. example code fragments)
• <ul><li> ... </li>... </ul> constructs can be used to list bullet points

...

Descriptions should avoid gratuitous capitalization and should avoid referencing class-names unless part of a @link pragma.

...

Note that parameter types and return types should not be referenced with @link pragmas in the method description as the javadoc compiler will automatically provide hyperlinks to those entities in the constructed documentation page (see example below).

• Internal comments ( // or /* ... */ ) are encouraged to provide additional context to the reader, where the nuances may not be discerned from the code alone.
• Avoid adding comments that provide no additional information, for example:
  • // sets the parent value on the node
  • node.setParent(parent);
• Avoid adding comments at the end of a statement; place them above the line concerned.

/**
 * Representation of something with "foo" behavior.
 * <p>
 * This is Javadoc commenting for classes and interfaces.
 * Javadoc descriptions are full sentences using English prose.
 */
public interface Foo {
 
    /**
     * Returns true if the given parameter is within tolerance of the sample.
     * <p>
     * Additional detail about what the method does, and maybe constraints
     * or assumptions about input parameters should be provided in the 
     * method description here.
     *
     * @param param functions that take arguments have this
     * @return methods that return a value should indicate this
     */
    boolean sampleMethod(Integer param);
} 
 

At the time of this writing, various formats are used within the code. 

/**
 * 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;
    }	
} 

Sample Javadoc generated documentation page

Image Added

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.

Class and interface names whose names are based on acronyms such as IP, PCEP, BGP, etc. should continue to follow the standard camel-casing convention, e.g. IpAddress, PcepService, BgpMessage. This maintains readability and avoids confusion between names of classes and names of constants. There are a few exceptions to this in ONOS code-base, mostly in legacy code; new code should adhere to the camel-casing convention, however.

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, Methods, Fields and Variables

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 are intended to be instantiated as immutable objects (e.g. DefaultDevice) should have all class variables declared final.

Collections (Sets, Maps, etc) returned by getters should be immutable, or a copy of the existing Collection.

@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;
}

Constants

• Constant names should be all-uppercase-underline-delimited:

  Code Block
  language java
  private static final long TIMEOUT_MS = 2_000; private static final String APP_NAME = "org.onosproject.fooapp";

• Recommendation: Constant strings for exception messages start with E_ prefix:

  Code Block
  language java
  private static final String E_BAD_ARG = "Bad argument detected: {}";

Concurrency

• Avoid synchronized(this) and synchronized methods wherever possible.

• Opt for thread-safe objects such as ConcurrentMap, and if using synchronized, 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.

equals() and hashCode()

Any objects that are compared, or stored in any hash based data structure (e.g. HashSet, HashMap), should implement these methods. For objects that implement them, comparisons should use their equals() method, and not ==. An exception to this rule are Enums.

Final keyword

Variables and parameters should not be gratuitously decorated with the final keyword; (This is an out-dated convention from Java 1.1 era). Let the Java compiler do its work of optimizing the code for you.

The final keyword should also be used on class declarations to prevent classes from being extended. For example, utility classes or collections of constants.

On a related note, even though method parameters are not decorated with final, they should still be treated as final; it is considered bad coding practice to modify a parameter within the method. The same behavior can always be achieved with local variables.

public void badExample(SomeType type, SomeValue value) {
    if (value == null

/**
 * Representation of something with "foo" behavior.
 * <p>
 * This is Javadoc commenting for classes and interfaces.
 * Javadoc descriptions are full sentences using English prose.
 */
public interface Foo {
 
    /**
     * Returns true if the given parameter is within tolerance of the sample.
     * <p>
     * Additional detail about what the method does, and maybe constraints
     * or assumptions about input parameters should be provided in the 
     * method description here.
     *
     * @param param functions that take arguments have this
     * @return methods that return a value should indicate this
     */
    boolean sampleMethod(Integer param);
} 
 

At the time of this writing, various formats are used within the code. 

/**
 * 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;
    }	
} 

Sample Javadoc generated documentation page

Image Removed

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.

Class and interface names whose names are based on acronyms such as IP, PCEP, BGP, etc. should continue to follow the standard camel-casing convention, e.g. IpAddress, PcepService, BgpMessage. This maintains readability and avoids confusion between names of classes and names of constants. There are a few exceptions to this in ONOS code-base, mostly in legacy code; new code should adhere to the camel-casing convention, however.

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, Methods, Fields and Variables

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 are intended to be instantiated as immutable objects (e.g. DefaultDevice) should have all class variables declared final.

Collections (Sets, Maps, etc) returned by getters should be immutable, or a copy of the existing Collection.

@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;
}

...

• Constant names should be all-uppercase-underline-delimited:

  Code Block
  language java
  private static final long TIMEOUT_MS = 2_000; private static final String APP_NAME = "org.onosproject.fooapp";

• Recommendation: Constant strings for exception messages start with E_ prefix:

  Code Block
  language java
  private static final String E_BAD_ARG = "Bad argument detected: {}";

Concurrency

...

Opt for thread-safe objects such as ConcurrentMap, and if using synchronized, apply it to the structure that must be locked:

...

...

) {
        value = 

...

SomeValue.DEFAULT_VALUE;
    }
    

...

...
 
 
public void goodExample(SomeType type, SomeValue value) {
    SomeValue v = value != null ? value 

...

: SomeValue.DEFAULT_VALUE;
    ...

Naming

• JSON fields should be in camel case:

  Code Block
  { } } }

  Additionally, some structures such as Hazelcast's IMap have per-key locks.

equals() and hashCode()

Any objects that are compared, or stored in any hash based data structure (e.g. HashSet, HashMap), should implement these methods. For objects that implement them, comparisons should use their equals() method, and not ==. An exception to this rule are Enums.

Final keyword

Variables and parameters should not be gratuitously decorated with the final keyword. This is an out-dated convention from Java 1.1 era. The only time this is required is when the parameter is used as part of an anonymous class closure defines within the method. Otherwise, let the Java compiler/optimizer to its work. The final keyword should also be used to on class declarations to make objects of the class immutable.

Naming

• JSON fields should be in camel case:

  Code Block
  { "aliasIp": "10.1.2.3" }

  and not "alias_ip".

Logging

The codebase uses SLF4J for logging. DO NOT use System.out.*. 

The logger should be private final, and associated with the particular class:

import static org.slf4j.LoggerFactory.getLogger;
import org.slf4j.Logger;
...
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()) {

• "aliasIp": "10.1.2.3" }

  and not "alias_ip".

Logging

The codebase uses SLF4J for logging. DO NOT use System.out.*. 

The logger should be private final, and associated with the particular class:

import static org.slf4j.LoggerFactory.getLogger;
import org.slf4j.Logger;
...
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.

When logging parameterized messages, use {} substituttion, not string concatenation. That is, do NOT do this:

  log.debug("Error detected at line " + lineNo + ", position " + pos);

Instead, do this:

  log.debug("Error detected at line {}, position {}", lineNo, pos);

 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)

...