Page History

...

• 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:

  Wiki Markup
  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
  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.

...