Recommenders/CodingConventions

From Eclipsepedia

Jump to: navigation, search

Contents


Introduction

This document lists Java coding recommendations for Eclipse Code Recommenders Committers. This text is an excerpt of several sources(http://geosoft.no/development/javastyle.html, Effective Java, Clean Code, ...) but slightly adopted for the Code Recommenders project. The recommendations described here are based on established standards collected from a number of sources, individual experience, local requirements/needs, as well as suggestions given in. If available, examples and explanations are given. Otherwise the references on the books are obtained to allow further research (e.g., "Item NN:" is kept for recommendations taken from Effective Java).

For students, the referenced books are available at Software Technology group and can be lent.

Layout of the Recommendation.

The recommendations are grouped by topic and each recommendation is numbered to make it easier to refer to during reviews.

Layout for the recommendations is as follows:

  1. n. Guideline short description
  2. Example if applicable
  3. Motivation, background and additional information.

The motivation section is important. Coding standards and guidelines tend to start "religious wars", and it is important to state the background for the recommendation.

Recommendation Importance

In the guideline sections the terms must, should and can have special meaning. A must requirement must be followed, a should is a strong recommendation, and a can is a general guideline.

Automatic Style Checking

Many tools provide automatic code style checking and automated code formatting. Checkstyle by Oliver Burn is probably one of the most popular of such tools. An Eclipse integration for Checkstyle can be found here http://eclipse-cs.sourceforge.net.

To use Checkstyle with Code Recommenders guidelines please use this xml configuration file: <TODO> 

General Recommendation

Any violation to the guide is allowed if it enhances readability.

The main goal of the recommendation is to improve readability and thereby the understanding and the maintainability and general quality of the code. It is impossible to cover all the specific cases in a general guide and the programmer should be flexible.

No code without license header!

At Eclipse every java source code file (at least) needs an EPL license header BEFORE it is checked in. The typical license header looks like this:

/**
 * Copyright (c) 2010 Darmstadt University of Technology.
 * All rights reserved. This program and the accompanying materials
 * are made available under the terms of the Eclipse Public License v1.0
 * which accompanies this distribution, and is available at
 * http://www.eclipse.org/legal/epl-v10.html
 *
 * Contributors:
 *    Marcel Bruch - initial API and implementation.
 */
package org.eclipse.recommenders.internal.rcp.codecompletion;

import ...
public class CompilerAstCompletionNodeFinder extends ASTVisitor {
  ...
}

Adopt the copyright header and Contributors section according to your affiliation and name. Inform your build master if you are using a new license. Otherwise it may break the build.

No check-in without running mvn clean install locally.

$ pwd
/Users/Marcel/Workspaces/code-recommenders/public/org.eclipse.recommenders.parent
$ mvn clean install
... wait a few minutes and check the build results

The one who breaks the build... you already know what happens, right?

Naming Conventions

Project Naming Conventions

There are several kinds of projects:

  • Plug-in projects
  • Feature (source) projects
  • Test plug-in or fragment projects
  • Test fixture projects
  • Repository projects
  • Product projects
  • Releng projects (including target platform definitions etc.)

Finding appropriate names for projects is challenging. Eclipse defines many package naming conventions but only few project naming conventions. The guiding principle to name a plug-in is to name the project after its main package (yes, there should only be one, and yes, this one should be unique to all plug-ins). This section gives some additional naming conventions for Code Recommenders project. Note, that Code Recommenders have RCP as well as server plug-ins which makes the naming convention a bit harder.


Plug-in Naming Conventions

Plug-in names follow the standard Eclipse Naming conventions. That means that (i) the project name on disk is the same as the plug-in id and (ii) plug-ins are named after their primary package. We recommend to flag plug-ins with a platform flag. Plug-ins that are considered to run on a server platform should be flagged with server, plug-ins considered to run inside Eclipse RCP with rcp, and plug-ins that can run on any platform without any platform flag.

The general naming schema for plug-ins is as follows:

plug-in id:

    org.eclipse.recommenders.<component>[""|.rcp|.server][.*]


source plug-in id:

    org.eclipse.recommenders.<component>[""|.rcp|.server][.*].source 


platform flag:

    [""|.rcp|.server]


Examples:

org.eclipse.recommenders.utils
org.eclipse.recommenders.utils.gson
org.eclipse.recommenders.utils.rcp
org.eclipse.recommenders.utils.web (put the webclient in here?)

org.eclipse.recommenders.injection


org.eclipse.recommenders.jayes
org.eclipse.recommenders.bayesnet ???

"rcp core plugins - no component since these plug-ins offer central services":
org.eclipse.recommenders.rcp 
org.eclipse.recommenders.rcp.builder
org.eclipse.recommenders.rcp.store
org.eclipse.recommenders.rcp.completion

org.eclipse.recommenders.overrides.rcp
org.eclipse.recommenders.chain.rcp
org.eclipse.recommenders.subwords.rcp




org.eclipse.recommenders.analysis
org.eclipse.recommenders.analysis.rcp

org.eclipse.recommenders.codesearch (<-- implicit commons)
org.eclipse.recommenders.codesearch.rcp
org.eclipse.recommenders.codesearch.server

org.eclipse.recommenders.snipmatch (<-- implicit commons)
org.eclipse.recommenders.snipmatch.rcp
org.eclipse.recommenders.snipmatch.server

org.eclipse.recommenders.stacktrace (<-- implicit commons)
org.eclipse.recommenders.stacktrace.rcp
org.eclipse.recommenders.stacktrace.server

org.eclipse.recommenders.calls (<-- implicit commons)
org.eclipse.recommenders.calls.server ???
org.eclipse.recommenders.calls.rcp

Test Projects Naming Convention

Test projects are mostly fragments. They use the same naming conventions as plug-ins except that they use the tests flag right after the project name (i.e. org.eclipse.recommenders.tests). This practice is recommended by the Eclipse architecture council.

Schema:

tests project id:

    org.eclipse.recommenders.tests.<component>[.*]

source tests project id (pure virtual - used in source features at most):

    org.eclipse.<subproject>.tests.<component>[.*].source

Usually, these names look like the plug-in name they contribute tests for but with the additional tests flag. Examples:

org.eclipse.recommenders.tests.calls.rcp
org.eclipse.recommenders.tests.rcp.builder

Test projects may offer common test infrastructure that may be reused between several test projects. These general purpose project may declare their own namespaces but should consider using the commons or shared namespace, e.g., org.eclipse.recommenders.tests.commons or org.eclipse.recommenders.tests.shared.

Feature Project Naming Convention

To line up the names according to the tests convention, we decided to use the feature flag and put it (similar to tests) directly behind the org.eclipse.recommenders prefix.

Schema:

feature id: 

    org.eclipse.recommenders.feature.<component>[.*]


source feature id:

    org.eclipse.recommenders.feature.<component>[.*].source

Examples:

org.eclipse.recommenders.feature.calls.rcp
org.eclipse.recommenders.feature.calls.rcp.source

There may be additional feature packagings such as special packages for e4, rcp (==e3), server etc. The source flag is always the last segment in the feature id.

P2 Repository Projects

P2 repository projects should be prefixed with a repository flag.

Schema:


p2 repository id:

    org.eclipse.recommenders.repository.<distribution>.[variant]

Examples:

org.eclipse.recommenders.repository{.all}
org.eclipse.recommenders.repository.rcp
org.eclipse.recommenders.repository.e4
org.eclipse.recommenders.repository.server

Product Projects

Products are currently used to build easy-to-install server distributions allowing developers to create an in-house knowledge base using code recommenders. More such packages may follow. Product projects should be prefixed with a product flag as follows.

Schema:


product id:

    org.eclipse.recommenders.product.<distribution>.[variant]

Examples:

org.eclipse.recommenders.product.server

the distribution template should pickup the platform flag as specified for plug-ins and features.

Project Disk Layout

Git repositories should be structured following this layout:

/
 pom.xml (general settings such as versions, compiler settings etc.; group id=org.eclipse.recommenders)
 plugins/
     pom.xml (specific build settings)
     org.eclipse.recommenders.*
 features/
     pom.xml (if needed)
     org.eclipse.recommenders.feature.*
     org.eclipse.recommenders.feature.*.source
 tests/
     pom.xml (specific test settings)
     org.eclipse.recommenders.tests.*
 fixtures/
     org.eclipse.recommenders.fixture.*
 repositories/
     org.eclipse.recommenders.repository.*
 build/
     org.eclipse.recommenders.build.*
 products/
     org.eclipse.recommenders.product.*

Using flags such as feature, build, tests etc. seems redundant for project names given this disk layout. However, having all these projects in a single Eclipse workspace requires all projects to have a unique name. This is a bit redundant but I think an acceptable approach.

We considered using a per-component layout but this seems to be too much overhead given that our components consists of just a few plug-ins (sometimes just one project), and thus, are yet too small.

Code Naming Conventions

Names representing packages should be in all lower case.

 mypackage, com.company.application.ui 

Package naming convention used by Sun for the Java core packages. The initial package name representing the domain name must be in lower case.


Eclipse Package Naming Conventions

At Eclipse there are a set of naming conventions described in great detail here: Eclipse general naming conventions. This section summarizes them and add a few Code Recommenders specifc ones.


All packages start with:

org.eclipse.recommenders.*

Internal packages always start with

org.eclipse.recommenders.internal.* // NOT org.eclipse.recommenders.other.package.internal;
  // NOT org.eclipse.recommenders.other.internal.package;

Test packages start with

org.eclipse.recommenders.tests.* // NOT org.eclipse.recommenders.other.package.tests;
  // NOT org.eclipse.recommenders.other.tests.package;


Example packages start with

org.eclipse.recommenders.examples.* // NOT org.eclipse.recommenders.other.package.examples;
  // NOT org.eclipse.recommenders.other.examples.package;

General package naming recommendations

org.eclipse.recommenders.<component>[.*].wiring --> classes that contain glue code to configure and wire objects such as Google Guice modules, web service descriptors etc.
org.eclipse.recommenders.<component>[.*].net --> classes that contain mostly classes for network communication
org.eclipse.recommenders.tests.<component>[.*].interactive --> classes that contain interactive  (i.e., manual) test suites (Eclipse convention)

Names representing types must be nouns and written in mixed case starting with upper case.

Line, AudioSystem

Common practice in the Java development community and also the type naming convention used by Sun for the Java core packages.

Interfaces follow the Eclipse Style I[TypeName] convention

public interface IVariableUsageResolver // NOT just VariableUsageResolver
{
 ...
}

The I[TypeName] convention allows developers to quickly assess the relevant interfaces in a package. Like it or not - this is an Eclipse convention we have to obey.

Variable names must be in mixed case starting with lower case.

line, audioSystem

Common practice in the Java development community and also the naming convention for variables used by Sun for the Java core packages. Makes variables easy to distinguish from types, and effectively resolves potential naming collision as in the declaration Line line;

Names representing constants (final variables) must be all uppercase using underscore to separate words.

MAX_ITERATIONS, COLOR_RED

Common practice in the Java development community and also the naming convention used by Sun for the Java core packages.

TODO :Add section about enums

Names representing methods must be verbs and written in mixed case starting with lower case.

getName(), computeTotalWidth()

Common practice in the Java development community and also the naming convention used by Sun for the Java core packages. This is identical to variable names, but methods in Java are already distinguishable from variables by their specific form.

Abbreviations and acronyms should not be uppercase when used as name.

exportHtmlSource(); // NOT: exportHTMLSource();
openDvdPlayer(); // NOT: openDVDPlayer();

Using all uppercase for the base name will give conflicts with the naming conventions given above. A variable of this type whould have to be named dVD, hTML etc. which obviously is not very readable. Another problem is illustrated in the examples above; When the name is connected to another, the readability is seriously reduced; The word following the acronym does not stand out as it should.

TODO

liste der Standard abkürzungen:

ast -> jdt.deom rec -> recommenders jdt -> IJavaElement compiler -> compiler internal

cu -> compilationUnit -> preferred compilationUnit (no abbrev)

Private class variables must not have any field prefix or suffix.

class Person
{
  private String name; // NOT private String _name;
  // NOT private String fName;
// NOT private String name_;
  ...
}


void setName(String name)
{
  this.name = name; // NOT name_= name;
}

To eliminate misinterpretations qualify field puts with this. and use the same names. Use IDE highlighting settings to make more visible on which kind of variable you are working.

TODO

Generic variables should have the same name as their type.

void setTopic(Topic topic) // NOT: void setTopic(Topic value)
  // NOT: void setTopic(Topic aTopic)
  // NOT: void setTopic(Topic t) 

void connect(Database database) // NOT: void connect(Database db)
  // NOT: void connect(Database oracleDB)

Reduce complexity by reducing the number of terms and names used. Also makes it easy to deduce the type given a variable name only. If for some reason this convention doesn't seem to fit it is a strong indication that the type name is badly chosen.

Non-generic variables have a role. These variables can often be named by combining role and type:

Point startingPoint, centerPoint;
Name loginName;

All names should be written in English.

English is the preferred language for international development.

Variables with a large scope should have long names, variables with a small scope can have short names

Scratch variables used for temporary storage or indices are best kept short. A programmer reading such variables should be able to assume that its value is not used outside a few lines of code. Common scratch variables for integers are i, j, k, m, n and for characters c and d.

The name of the object is implicit, and should be avoided in a method name.

line.getLength(); // NOT: line.getLineLength();

The latter might seem natural in the class declaration, but proves superfluous in use, as shown in the example.

Specific Naming Conventions

The terms get/set must be used where an attribute is accessed directly.

employee.getName();
employee.setName(name);

matrix.getElement(2, 4);
matrix.setElement(2, 4, value);

Common practice in the Java community and the convention used by Sun for the Java core packages. Getters should not change the internal state of the object. Exceptions may be lazy initializers. Getter should actually return something :-)

NOTE: this is a pure GET. No real computation effort needed.

is prefix should be used for boolean variables and methods.

isSet, isVisible, isFinished, isFound, isOpen

This is the naming convention for boolean methods and variables used by Sun for the Java core packages. Using the is prefix solves a common problem of choosing bad boolean names like status or flag. isStatus or isFlag simply doesn't fit, and the programmer is forced to chose more meaningful names.

Setter methods for boolean variables must have set prefix as in:

void setFound(boolean isFound);

There are a few alternatives to the is prefix that fits better in some situations. These are has, can and should prefixes:

boolean hasLicense();
boolean canEvaluate();
boolean shouldAbort = false;

The term compute can be used in methods where something is computed.

valueSet.computeAverage();
matrix.computeInverse()

Give the reader the immediate clue that this is a potential time consuming operation, and if used repeatedly, he might consider caching the result. Consistent use of the term enhances readability.

The term find can be used in methods where something is looked up.

vertex.findNearestVertex();
matrix.findSmallestElement();
node.findShortestPath(Node destinationNode);

Give the reader the immediate clue that this is a simple look up method with a minimum of computations involved. Consistent use of the term enhances readability.

The term initialize can be used where an object or a concept is established.

printer.initializeFontSet();

The American initialize should be preferred over the English initialise. Abbreviation init should be avoided.


The use of initializeXYZ() in constructors to initialize complex fields is preferred over direct field initalizations.

Set<Statements> supportedCompletionRequests;
  // NOT Set<Statements> supportedCompletionNodes = createSupportedCompletionRequest()

public CallsCompletionEngine(..) {
  ...
  initializeSuportedCompletionRequests();
}

private void initializeSuportedCompletionRequests() 
{
  supportedCompletionRequests = Sets.newHashSet();
  supportedCompletionRequests.add(CompletionOnMemberAccess.class);
  supportedCompletionRequests.add(CompletionOnMessageSend.class);
  supportedCompletionRequests.add(CompletionOnQualifiedNameReference.class);
  supportedCompletionRequests.add(CompletionOnSingleNameReference.class);
}

JFC (Java Swing) and SWT variables should be suffixed by the element type.

widthScale, nameTextField, leftScrollbar, mainPanel, fileToggle, minLabel, printerDialog

Enhances readability since the name gives the user an immediate clue of the type of the variable and thereby the available resources of the object.

Plural form should be used on names representing a collection of objects.

Collection<Point> points;
int[] values;

Enhances readability since the name gives the user an immediate clue of the type of the variable and the operations that can be performed on its elements.

numberOf prefix should be used for variables representing a number of objects.

numberOfPoints, numberOfLines

Note that Sun use num prefix in the core Java packages for such variables. This is probably meant as an abbreviation of number of, but as it looks more like number it makes the variable name strange and misleading. If "number of" is the preferred phrase, numberOf prefix can be used instead of just n. num prefix must not be used.


Iterator variables should be called it

for (Iterator it = points.iterator(); it.hasNext(); )
{
  ...
} 

for (int i = 0; i < numberOfTables; i++)
{
  ...
}

Complement names must be used for complement entities.

 
get/set, add/remove, create/destroy, start/stop, insert/delete,
increment/decrement, old/new, begin/end, first/last, up/down, min/max,
next/previous, old/new, open/close, show/hide, suspend/resume, etc.

Reduce complexity by symmetry.

Abbreviations in names should be avoided.

computeAverage(); // NOT: compAvg();
ActionEvent event; // NOT: ActionEvent e;
catch (Exception exception) { // NOT: catch (Exception e) {

There are two types of words to consider. First are the common words listed in a language dictionary. These must never be abbreviated. Never write:

cmd instead of command
comp instead of compute
cp instead of copy
e instead of exception
init instead of initialize
pt instead of point
etc.

Then there are domain specific phrases that are more naturally known through their acronym or abbreviations. These phrases should be kept abbreviated. Never write:

HypertextMarkupLanguage instead of html
CentralProcessingUnit instead of cpu
PriceEarningRatio instead of pe
etc.

TODO: cu, exception changed

Negated boolean variable names must be avoided.

bool isError; // NOT: isNoError
bool isFound; // NOT: isNotFound

The problem arise when the logical not operator is used and double negative arises. It is not immediately apparent what !isNotError means.

Associated constants (final variables) should be prefixed by a common type name.

final int COLOR_RED = 1;
final int COLOR_GREEN = 2;
final int COLOR_BLUE = 3;

This indicates that the constants belong together, and what concept the constants represents.

An alternative to this approach is to put the constants inside an interface effectively prefixing their names with the name of the interface:

interface Color
{
  final int RED = 1;
  final int GREEN = 2;
  final int BLUE = 3;
}


Exception classes should be suffixed with Exception.

class AccessException extends Exception{...}

Exception classes are really not part of the main design of the program, and naming them like this makes them stand out relative to the other classes. This standard is followed by Sun in the basic Java library.

General Programming

Creating and Destroying Objects

Item 1: Consider static factory methods instead of constructors

see "Effective Java" for details.

Item 2: Consider a builder when faced with many constructor parameters

see "Effective Java" for details.

Item 3: Enforce the singleton property with a private constructor or an enum type

see "Effective Java" for details.

Item 4: Enforce noninstantiability with a private constructor

see "Effective Java" for details.

Item 5: Avoid creating unnecessary objects

see "Effective Java" for details.

Item 6: Eliminate obsolete object references

see "Effective Java" for details.

Item 7: Avoid finalizers

see "Effective Java" for details.

Methods Common to All Objects

Item 8: Obey the general contract when overriding equals

see "Effective Java" for details.

Item 9: Always override hashCode when you override equals

see "Effective Java" for details.

Item 10: Always override toString

see "Effective Java" for details. === Item 11: Override clone judiciously

Item 12: Consider implementing Comparable

see "Effective Java" for details.

Generics

Item 23: Don’t use raw types in new code

see "Effective Java" for details.

Item 24: Eliminate unchecked warnings

see "Effective Java" for details.

Item 25: Prefer lists to arrays

see "Effective Java" for details.

Item 26: Favor generic types

see "Effective Java" for details.

Item 27: Favor generic methods

see "Effective Java" for details.

Item 28: Use bounded wildcards to increase API flexibility

see "Effective Java" for details.

Item 29: Consider typesafe heterogeneous containers

see "Effective Java" for details.


General Programming Style

Item 45: Minimize the scope of local variables

see "Effective Java" for details.

Item 46: Prefer for-each loops to traditional for loops

see "Effective Java" for details.

Item 47: Know and use the libraries

see "Effective Java" for details.

Item 48: Avoid float and double if exact answers are required

see "Effective Java" for details.

Item 49: Prefer primitive types to boxed primitives

see "Effective Java" for details.

Item 50: Avoid strings where other types are more appropriate

see "Effective Java" for details.

Item 51: Beware the performance of string concatenation

see "Effective Java" for details.

Item 52: Refer to objects by their interfaces

see "Effective Java" for details.

Item 53: Prefer interfaces to reflection

see "Effective Java" for details.

Item 54: Use native methods judiciously

see "Effective Java" for details.

Item 55: Optimize judiciously

see "Effective Java" for details.

Item 56: Adhere to generally accepted naming conventions

see "Effective Java" for details.


Exceptions

Item 57: Use exceptions only for exceptional conditions

see "Effective Java" for details.

Item 58: Use checked exceptions for recoverable conditions and runtime exceptions for programming errors

see "Effective Java" for details.

Item 59: Avoid unnecessary use of checked exceptions

see "Effective Java" for details.

Item 60: Favor the use of standard exceptions

see "Effective Java" for details.

Item 61: Throw exceptions appropriate to the abstraction

see "Effective Java" for details.

Item 62: Document all exceptions thrown by each method

see "Effective Java" for details.

Item 63: Include failure-capture information in detail messages

see "Effective Java" for details.

Item 64: Strive for failure atomicity

see "Effective Java" for details.

Item 65: Don’t ignore exceptions

see "Effective Java" for details.



Statements

Package and Import Statements

The package statement must be the first statement of the file. All files should belong to a specific package.

The package statement location is enforced by the Java language. Letting all files belong to an actual (rather than the Java default) package enforces Java language object oriented programming techniques.

=== The import statements must follow the package statement. import statements should be sorted with the most fundamental packages first, and grouped with associated packages together and one blank line between groups.

import java.io.IOException;
import java.net.URL;

import java.rmi.RmiServer;
import java.rmi.server.Server;

import javax.swing.JPanel;
import javax.swing.event.ActionEvent;

import org.linux.apache.server.SoapServer;


The import statement location is enforced by the Java language. The sorting makes it simple to browse the list when there are many imports, and it makes it easy to determine the dependiencies of the present package The grouping reduce complexity by collapsing related information into a common unit.

Your IDE supports you on this. Check out your settings.

Imported classes should always be listed explicitly.

import java.util.List;      // NOT: import java.util.*;
import java.util.ArrayList;
import java.util.HashSet;

Importing classes explicitly gives an excellent documentation value for the class at hand and makes the class easier to comprehend and maintain.

Your IDE will always keep the import list minimal and up to date - if you enable this feature in your preferences.

Classes and Interfaces

Class and Interface declarations should be organized in the following manner:

Class/Interface documentation.
class or interface statement.
Class (static) variables in the order public, protected, package (no access modifier), private.
Instance variables in the order public, protected, package (no access modifier), private.
Constructors.
Methods (in order they are used in code AKA vertical density).

Reduce complexity by making the location of each class element predictable.

Item 13: Minimize the accessibility of classes and members

see "Effective Java" for details.

Item 14: In public classes, use accessor methods, not public fields

see "Effective Java" for details.

Item 15: Minimize mutability

see "Effective Java" for details.

Item 16: Favor composition over inheritance

see "Effective Java" for details.

Item 17: Design and document for inheritance or else prohibit it

see "Effective Java" for details.

Item 18: Prefer interfaces to abstract classes

see "Effective Java" for details.

Item 19: Use interfaces only to define types

see "Effective Java" for details.

Item 20: Prefer class hierarchies to tagged classes

see "Effective Java" for details.

Item 21: Use function objects to represent strategies

see "Effective Java" for details.

Item 22: Favor static member classes over nonstatic

see "Effective Java" for details.


Enums and Annotations

Item 30: Use enums instead of int constants

see "Effective Java" for details.

Item 31: Use instance fields instead of ordinals

see "Effective Java" for details.

Item 32: Use EnumSet instead of bit fields

see "Effective Java" for details.

Item 33: Use EnumMap instead of ordinal indexing

see "Effective Java" for details.

Item 34: Emulate extensible enums with interfaces

see "Effective Java" for details.

Item 35: Prefer annotations to naming patterns

see "Effective Java" for details.

Item 36: Consistently use the Override annotation

see "Effective Java" for details.

Item 37: Use marker interfaces to define types

see "Effective Java" for details.


Methods

Methods should be small!

The first rule of methods is that they should be small. The second rule of methods is that 'they should be smaller than that!

This brings up another question: 20 lines isn't too large, right? Here at code recommenders we would say "No, a method with 20 lines is too large." Methods typically can be broke down to smaller chunks containing of 2,3, or 4 methods each. Sounds strange but the large a method the higher the probability that your method does many different things and thus would violate the next guideline. Thus, if you have a method that is larger than 5 lines carefully look on your code and be sure that this piece of work cannot be divided further into smaller pieces.


Methods should do one thing and one thing only

From Clean Code: Methods should do one thing. And they should do it well. They should do 'only this one thing.

TODO: provide a code snippet

API Methods should never return null!

    // bad: 
    public IJavaElement getEnclosing(){
          return hasElement? elem: null;
    }

    // better:
    public Optional<IJavaElement> getEnclosing(){
          return Optional.fromNullable(elem);
         // or: Optional.absent()
    }

Developers tend to assume that methods never return null. This is the source of most NullPointerExceptions. Thus, if a method may return null consider using Optional<T> classes as provided by Google Guava 10.0. Code Recommenders currently has it's own class for this (org.eclipse.recommenders.utils.Option) but this will be replaced soon. This guideline applies for public and protected methods that may be called from external clients. Dealing with null values internally is up to the developer. It's not enforced to apply this guideline to private methods too - it may or may not make sense depending on your case.

If there are methods such as hasX(), you may consider to throw an exception if getX() would return null. Not sure yet, which approach is more favorable. We are currently one-by-one changing our APIs to use optional classes.

Method modifiers should be given in the following order:

<annotations> <access> abstract static final synchronized native strictfp

Following any <annotations> (like @Overide or @Nullable), which are not real modifiers, the <access> modifier (if present, one of public, protected or private) must be the first modifier.

public static double square(double a);  // NOT: static public double square(double a);

The most important lesson here is to keep the access modifier as the first modifier. Of the possible modifiers, this is by far the most important, and it must stand out in the method declaration. For the other modifiers, the order is less important. Nevertheless, the suggestions of the Java Language Specification on modifier order should be followed.

Item 38: Check parameters for validity

see Effective Java for details.

Item 39: Make defensive copies when needed

see Effective Java for details.

Item 40: Design method signatures carefully

see Effective Java for details.

Item 41: Use overloading judiciously

see Effective Java for details.

Item 42: Use varargs judiciously

see Effective Java for details.

Item 43: Return empty arrays or collections, not nulls

see Effective Java for details.

Item 44: Write doc comments for all exposed API elements

see Effective Java for details.


Variables

Type conversions must always be done explicitly. Never rely on implicit type conversion.

 floatValue = (int) intValue; // NOT: floatValue = intValue; 

By this, the programmer indicates that he is aware of the different types involved and that the mix is intentional.

Array specifiers must be attached to the type not the variable.

 int[] a = new int[20];   // NOT: int a[] = new int[20]

The arrayness is a feature of the base type, not the variable. It is not known why Sun allows both forms.

Prefer primitive types over their wrapping types

void m(Boolean b){ // NOT:
    if(!b.booleanValue())
        // do something;
    }

void m(boolean b){ // BETTER:
    if(!b)
        // do something;
    }

You should use primitive types instead of their wrapping types whenever possible. When using the wrapping type, we assume that null is a valid and possible value that must be handled. Favor wrapper types if conversion between primitive and object type (int vs. Integer) would otherwise occur very frequently.

Variables should be initialized where they are declared and they should be declared in the smallest scope possible.

This ensures that variables are valid at any time. Sometimes it is impossible to initialize a variable to a valid value where it is declared. In these cases it should be left uninitialized rather than initialized to some phony value.

m(){ // NOT
    List<C> res = Collections.emptyList();
    if(someExpression){
        ...
        res = new ArrayList();
        .... // 10 lines more
    }
    return res;
}

Better:

m(){
    if(someExpression){
        ...
        List<C> res = new ArrayList();
        .... // 10 lines more
        return res;
    }
    return Collections.emptyList();
}

Variables must never have dual meaning.

Enhances readability by ensuring all concepts are represented uniquely. Reduce chance of error by side effects.

Class variables should never be declared public.

The concept of Java information hiding and encapsulation is violated by public variables. Use private variables and access functions instead. One exception to this rule is when the class is essentially a data structure, with no behavior (equivalent to a C++ struct). In this case it is appropriate to make the class' instance variables public [2].

Arrays should be declared with their brackets next to the type.

double[] vertex;  // NOT: double vertex[];
int[]    count;   // NOT: int    count[];

public static void main(String[] arguments)

public double[] computeVertex()

The reason for is twofold. First, the array-ness is a feature of the class, not the variable. Second, when returning an array from a method, it is not possible to have the brackets with other than the type (as shown in the last example).

Variables should be kept alive for as short a time as possible.

Keeping the operations on a variable within a small scope, it is easier to control the effects and side effects of the variable.

Loops

Only loop control statements must be included in the for() construction.

sum = 0;                       // NOT: for (i = 0, sum = 0; i < 100; i++)
for (i = 0; i < 100; i++)                sum += value[i];
  sum += value[i];

Increase maintainability and readability. Make a clear distinction of what controls and what is contained in the loop.

Loop variables should be initialized immediately before the loop.

isDone = false;           // NOT: bool isDone = false;
while (!isDone) {         //      :
  :                       //      while (!isDone) {
}                         //        :
                          //      }

The use of do-while loops can be avoided.

do-while loops are less readable than ordinary while loops and for loops since the conditional is at the bottom of the loop. The reader must scan the entire loop in order to understand the scope of the loop. In addition, do-while loops are not needed. Any do-while loop can easily be rewritten into a while loop or a for loop. Reducing the number of constructs used enhance readability.

The use of break and continue in loops should be avoided.

These statements should only be used if they prove to give higher readability than their structured counterparts.


Conditionals

Complex conditional expressions must be avoided. Introduce temporary boolean variables or selfexplaining methods instead.

bool isFinished = (elementNo < 0) || (elementNo > maxElement);
bool isRepeatedEntry = elementNo == lastElement;
if (isFinished || isRepeatedEntry) {
  :
}


// NOT:
if ((elementNo < 0) || (elementNo > maxElement)||
     elementNo == lastElement) {
  :
}


// even better:
if (isFinished(..) || isRepeatedEntry(..)) {
  :
}

By assigning boolean variables to expressions, the program gets automatic documentation. The construction will be easier to read, debug and maintain. Using methods that reveal the intension of these checks should be preferred over use of temporary variables!


The nominal case should be put in the if-part and the exception in the else-part of an if statement.

boolean isOk = readFile(fileName);
if (isOk) {
  :
}
else {
  :
}

Makes sure that the exceptions does not obscure the normal path of execution. This is important for both the readability and performance.


The conditional should be put on a separate line enclosed by curly brackets.

if (isDone)
{       // NOT: if (isDone) doCleanup();
  doCleanup();
}

This is for debugging purposes. When writing on a single line, it is not apparent whether the test is really true or not. Also the usage of curly brackets ensure that no statements accidentally fall out of an if branch:

if (isDone)       
  doCleanup();
  unregisterListener(); // unregister will always be called - even if !isDone

Executable statements in conditionals must be avoided.

InputStream stream = File.open(fileName, "w");
if (stream != null) {
  :
}

// NOT:
if (File.open(fileName, "w") != null)) {
  :
}

Conditionals with executable statements are simply very difficult to read. This is especially true for programmers new to Java.

Miscellaneous

The use of magic numbers in the code should be avoided. Numbers other than 0 and 1 can be considered declared as named constants instead.

private static final int  TEAM_SIZE = 11;
:
Player[] players = new Player[TEAM_SIZE]; // NOT: Player[] players = new Player[11];
If the number does not have an obvious meaning by itself, the readability is enhanced by introducing a named constant instead.
58. Floating point constants should always be written with decimal point and at least one decimal.
double total = 0.0;    // NOT:  double total = 0;
double speed = 3.0e8;  // NOT:  double speed = 3e8;

double sum;
:
sum = (a + b) * 10.0;

This emphasize the different nature of integer and floating point numbers. Mathematically the two model completely different and non-compatible concepts.

Also, as in the last example above, it emphasize the type of the assigned variable (sum) at a point in the code where this might not be evident.

Floating point constants should always be written with a digit before the decimal point.

double total = 0.5;  // NOT:  double total = .5;

The number and expression system in Java is borrowed from mathematics and one should adhere to mathematical conventions for syntax wherever possible. Also, 0.5 is a lot more readable than .5; There is no way it can be mixed with the integer 5.

Static variables or methods must always be refered to through the class name and never through an instance variable.

 Thread.sleep(1000);    // NOT: thread.sleep(1000); 

This emphasize that the element references is static and independent of any particular instance. For the same reason the class name should also be included when a variable or method is accessed from within the same class.

Default interface implementations can be prefixed by Default.

class DefaultTableCellRenderer implements TableCellRenderer {...}

It is not uncommon to create a simplistic class implementation of an interface providing default behaviour to the interface methods. The convention of prefixing these classes by Default has been adopted by Sun for the Java library.

Singleton classes should return their sole instance through method getInstance.

 
class UnitManager
{
  private final static UnitManager instance_ = new UnitManager(); 

  private UnitManager() {...} 

  public static UnitManager getInstance() // NOT: get() or instance() or unitManager() etc.
  {
    return instance_;
  }
}

Common practice in the Java community though not consistently followed by Sun in the JDK. The above layout is the preferred pattern.

Classes that creates instances on behalf of others (factories) can do so through method new[ClassName]

class PointFactory
{
  public Point newPoint(...)
  {
    ...
  }
}

Indicates that the instance is created by new inside the factory method and that the construct is a controlled replacement of new Point().

Consider static factory methods instead of constructors

Advantages:

  1. One advantage of static factory methods is that, unlike constructors, they have names.
  2. A second advantage of static factory methods is that, unlike constructors, they are not required to create a new object each time they’re invoked.
  3. A third advantage of static factory methods is that, unlike constructors, they can return an object of any subtype of their return type.

Disadvantages:

  1. The main disadvantage of providing only static factory methods is that classes without public or protected constructors cannot be subclassed.
  2. A second disadvantage of static factory methods is that they are not readily distinguishable from other static methods.

Follow a set of conventions when using them:

  • valueOf - Returns an instance that has, loosely speaking, the same value as its parameters. Such static factories are effectively type-conversion methods. Example: String.valueOf(boolean);
  • of - A concise alternative to valueOf, popularized by EnumSet.
  • getInstance - Returns an instance that is described by the parameters but cannot be said to have the same value. In the case of a singleton, getInstance takes no parameters and returns the sole instance.
  • newInstance - Like getInstance, except that newInstance guarantees that each instance returned is distinct from all others.
  • getType - Like getInstance, but used when the factory method is in a different class. Type indicates the type of object returned by the factory method.
  • newType - Like newInstance, but used when the factory method is in a different class. Type indicates the type of object returned by the factory method.
  • create[SomeSuffix] - Like newType, but may perform some special initializations on the returned objects. Example: Type.createWithDefaultResolver(...);

Public methods may delegate work to private/protected do[MethodName] methods and enclose their invocation with try/catch

public void append(..)
{
  try
  {
    doAppend(..);
  } 
  catch (Exception e)
  {
    logAppendFailedError(e);
  }
}

Sometimes the code to execute may throw exceptions which should be catched and not rethrown. Also subclasses may reimplement some behavior of the algorithm. To ensure consistency with the API one may delegate to a do[MethodName]() method and make the public method final. This way the superclass can ensure some consistency independent of actual subclasses executed.

Functions (methods returning an object) should be named after what they return and procedures (void methods) after what they do.

Increase readability. Makes it clear what the unit should do and especially all the things it is not supposed to do. This again makes it easier to keep the code clean of side effects.

Layout and Comments

Layout

Code Recommenders provides a code formatter template for Eclipse which must be used by all committers. It's located in the releng project.

Comments

Most conventions on comments are enforced by the code formatter template of the code recommenders project. Thus just a few general notes on comments here.

Tricky code should not be commented but rewritten.

In general, the use of comments should be minimized by making the code self-documenting by appropriate name choices and an explicit logical structure.

All comments should be written in English.

In an international environment English is the preferred language.


Use // for all non-JavaDoc comments, including multi-line comments.

// Comment spanning
// more than one line.

Since multilevel Java commenting is not supported, using // comments ensure that it is always possible to comment out entire sections of a file using /* */ for debugging purposes etc.


The declaration of anonymous collection variables should be followed by a comment stating the common type of the elements of the collection.

private Multimap<String /* simple class name*/, String /* full qualified class names*/>  simple2fullyQualifiedClassnames;

Without the extra comment it can be hard to figure out what the collection consist of, and thereby how to treat the elements of the collection. In methods taking collection variables as input, the common type of the elements should be given in the associated JavaDoc comment. Whenever possible one should of course qualify the collection with the type to make the comment superflous:

List of accepted acronyms

cu
Abbreviation for Compilation Unit. Although accepted the usage of compilationUnit should be preferred over cu.
ast
Abbreviation for Abstract Syntax Tree.
jdt
Abbreviation for Java Development Tools - also used if IJavaElements are used to make clear the difference to code recommenders or ast constructs with similar names.
rec
Abbreviation for Recommenders. Typically used as prefix if objects of similar names are used in the same context. Example: jdtCu vs. recCu


Something left over?

Send comments to recommenders-dev@eclipse.org