Coding Standards
From GEANT2-JRA1 Wiki
[edit]
Source Code Control
- We will be using SVN hosted by Internet2. SVN usage details
- Developers are expected to use SVN as described in Basic Work Cycle.
- Individual check-in's should relate to a single addition of functionality.
- Individuals should check-in frequently. If a single bit of functionality will take many check-in's to complete the unit of work, they should create an svn branch for the work, and then merge that branch into the main-line upon completion of the feature. (They can optionally delete the 'work' branch when they are done with it.)
- SVN alerts can be email'd to all interested parties. A summary of changes can be generated and emailed to the list on some regular schedule. (weekly?)
- Release tags should be created similar to the way FreeBSD uses CVS tags to support new releases. Examples of this can be found at www.freebsd.org.
[edit]
Code Format
- Style of code
- Usage of brackets
- Opening brackets should follow in the same line as the method/class/condition/etc and should be separated from it by a single white space character
- Usage of brackets
Example
try {
if (index > 10) {
....
} else {
....
}
} catch (Exception ex) {
....
}
- Grouping methods and member variables
- Declaration of all member variables should come first in the class definition
- All methods and operations should follow later
- A line composed of - character starting with // characters and ending with some text as in example should be used to separate private, public and abstract methods and operations
- Two empty lines should separate different sections of the code (sections that can be seen as code blocks, ex: between two operations, between an operation and member variables, etc)
Example
public class TestClass {
// ---------------------------------- constants
/**
* ....
*/
public static final String TYPE1 = "Type1";
/**
* ....
*/
public static final String TYPE2 = "Type2";
// ---------------------------------- class fields
/**
* ....
*/
private String type = null;
// ---------------------------------- constructors
/**
* ....
*/
public TestClass(String type) {
this.type = type;
}
// ---------------------------------- public methods
/**
* ....
*/
public ClassX method1()
....
}
// ---------------------------------- protected methods
/**
* ....
*/
protected ClassY method2() {
....
}
// ---------------------------------- private methods
/**
* ....
*/
private ClassZ method3() {
....
}
}
- Usage of tabs and whitespaces
- Tabs should not be used as different editors (and different environments) interpret them differently.
- Wherever a tab needs to be used, they should be replaced with 4 white spaces
- Usage of tabs and whitespaces
- Number of characters in a line
- The number of characters in a line should be limited to 80 as specified by Sun Coding guildelines
- If number of characters exceeds the above number, they should be split appropriately.
- Number of characters in a line
Example 1
Data data = (RRDDataList)
rrdFetch((RRDFile)metaData, (RRDFilterParameters)filterParameters);
Example 2
throw new SystemException(
"Cannot retrieve data from rrdjtool library: "
+ se.getMessage());
[edit]
Naming Convention
- Naming convention
- Java Convention of naming variables, classes and methods/operations should be followed
- Verbs should be used for operations/methods and they should begin with a lowerlcase letter and every following word should have the first letter in uppercase (see Example)
- Nouns should be used for classes. They should always begin with an uppercase letter and have each following word's first letter in uppercase (see example)
- Inheritance and Implementations should bear some sort of similarity with the parent/Interface class
- Java Convention of naming variables, classes and methods/operations should be followed
Example
public Interface InformationReader {
public Information getInformation();
public void dealWithInformation();
}
public class RRDTypeInformationReader extends InformationReader {
....
....
....
}
- Static member variables and member variables which are not static but whose value will not change should be named completely in upper cases
- Underscores should be used to separate multiple words in such names
- Static member variables and member variables which are not static but whose value will not change should be named completely in upper cases
Example
public interface Consolidation {
// -------------------------------------- constants
/**
* ....
*/
public static final String AVERAGE = "AVERAGE";
/**
* ....
*/
public static final String MIN = "MIN";
/**
* ....
*/
public String TYPE = "SOME_TYPE";
}
[edit]
Imports
- Only necessary classes should be imported
- A *, as in example below, should not be used. i.e., all classes should not be imported when only one or a few are necessary.
- It is suggested that imports should be grouped together based on the package i.e., imports for particular packages and imports of all their sub-packages should be together
Example: (Do not use) import org.perfsonar.ns.perfsonar.*;
Example: import org.ggf.ns.nmwg.test; import org.ggf.ns.nmwg.time.test import org.ggf.ns.nmwg.parameter.test; import org.perfsonar.ns.perfsonar.test; import org.perfsonar.ns.perfsonar.parameter.test;
[edit]
Documentation
- Reference documentation should be done using the in-line javadoc documentation tool. Following are some guidelines
- All java files should contain file metadata (this is not javadoc)
Example: /* * Created on Jul 15, 2005 * Version Number: $Id$ * Project : perfSONAR */
- All methods/operations should have javadoc comments
- Comments should contain at least
- Description of method
- @parameter (as many as applicable)
- @returns
- @throws (if applicable)
- @see any links to other methods and classes. ex: exception classes, other relavant classes
- Above comments should clearly specify what can be expected out of a method. Ex. Whether the method returns null, whether the method returns empty objects when data is not present, etc
- Comments should contain at least
Example:
/**
* Method to accept requests for action. It implements the method definition
* in ServiceEngine Interface
*
* @return ServiceEngineResponse object containing the response
* @throws RequestException
* @see org.perfsonar.service.commons.engine.ServiceEngine#takeAction(java.lang.String,
* org.perfsonar.service.commons.engine.ServiceEngineRequest)
* @see org.perfsonar.service.commons.ServiceEngineRequest
* @see org.perfsonar.service.commons.ServiceEngineResponse
* @see org.perfsonar.service.commons.exceptions.SystemException
* @see org.perfsonar.service.commons.exceptions.RequestException
*/
public ServiceEngineResponse takeAction(String actionType, ServiceEngineRequest request)
throws SystemException, RequestException {
}
- All classes should have javadoc comments
- Comments should contain at least
- Description of class
- @author Author Name/s who have contributed to the code
- @see if it is related (inheritance, implementation, etc) to any other class
- Comments should contain at least
Example:
/**
* Class which provides Network related utilities
*
* @author loukik
*
*/
public class NetUtil {
}
- Some member variables should have javadoc comments
[edit]
Commenting
- Lots of comments in code.
- Comment about a line should be before the line
- Comments written should mention reasons for actions taken in the code i.e., comments should be used to provide justification of code
- Comments should also be written in order to provide the pseudocode
- trivial bits of the code which call for trivial pseudocode need not be commented.
Examples: Justification // there is a need to prioritize these requests as each request needs a lot of resources and they should be awared first to ones with high priority highPriorityValue = prioritize(request1, request2) Example: Psuedocode // calculate mean of the two measurements double meanValue = mean(measurement1, measurement2); // store value into a database database.store(meanValue);
[edit]
Logging
- Levels of logging: Four levels/types of logging are available
- Level 1: Fatal
- Level 2: Error
- Level 3: Warn
- Level 4: Info
- Level 5: Debug
- Appropriate levels should be used where necessary
- Fatal logging should be used whenever fatal errors/exceptions are encountered
- Error logging should be used whenever non-fatal errors/exceptions are encountered
- Warn logging should be used whenever there is potential for errors. For example: low system resources
- Info logging should be 'conservatively' used whenever information such as state of the system, etc. have to be logged
- Debug logging should be used for debugging code
- Debug logging can sometimes aid in checking normal operation of the system
- Log appropriately to help in localizing errors
- Appropriate logging mainly in case of fatal and error levels should be done with the view to help localisation of errors
- Example which illustrates how to get an instance of the logger
import org.perfsonar.commons.messages.Log
org.perfsonar.commons.messages.Log logger = Log.getInstance();
logger.fatal("RRDServiceEngine: this is a fatal log");
logger.error("RRDServiceEngine: this is an error log");
logger.warn("RRDServiceEngine: this is a warning log");
logger.debug("RRDServiceEngine: this is a debug log");
logger.info("RRDServiceEngine: this is an info log");
[edit]
Errors and Exception Handling
- Usage of exceptions and Nesting exceptions should be based on design decisions
- All cases of exceptions should be handled and converted into defined set of exceptions
- When an exception (fatal or non-fatal) is encountered for the first time and is being converted into a locally defined exception set, they have to be logged (either as fatal or error). When this exception gets passed back subsequently, no logging is necessary at each subsequent level but only at its origin.
- All fatal exceptions should be thrown after any necessary conversion into locally defined exception set
- All non-fatal exceptions should be handled as per design
- Exception classes will contain Throwable object which provides stack trace.
- When an exception occurs which is not part of the defined list of exceptions, this exception should be nested inside the appropriate exception from the defined list by using the appropriate constructor (which contains the throwable parameter). This allows for preservation and supply of stack trace.
- When an exception has to be thrown by the system not because another internal exception was caused but for someother reason, stack trace is not necessary (as its not available).
- Regardless of the cause of exception (internal exception or some particular reason), following are necessary
- Appropriate message (message structure to be defined later)
- Location of errors (in two dimensions : the service/client in which exception happened, the class in which exception happened).
- Usage of available Exception Types
- Request Exception
- This exception type should be used whenever there is an error within the user's request itself. It can also be used when the request is ok (compliant to the schema) but the service does not have the capability to satisfy it
- DataFormatException
- This exception type should be used when the measurement data got either from a tool or directly has errors or is in a format which cannot be understood
- SystemException
- Whenever an error occurs because of the internal working of the system i.e., factors related to the system itself, system exception should be thrown
- ResourceException
- When a resource being requested cannot be satisfied, A ResourceException should be thrown.
- Example: When authorization fails or if the authorization level is insufficient for a request operation or if all resources have been scheduled already
- AuthenticationException
- When errors occur while authenticating a host or reading authentication information of a requestor, AuthenticationException should be used
- Request Exception
Example 1 (Exception as a result of another internal exception)
try {
// some code here
} catch (FileNotFoundException fe){
SystemException sysException = new SystemException("Required config file "+fileName+" was not found on the system", fe);
logger.fatal("RRDTypeMAServiceEngine: File containing config info "+fileName+" was not found on the system");
throw sysException;
}
Example 2 (Exception as a result of some condition and no other internal exception)
if(InsufficientPrivileges){
ResourceException resException = new ResourceException
("Privileges not sufficient to carry out required measurement operation");
logger.fatal("SNMPTypeResourceProtector: Client has insufficient privileges for the operation requested "+someInfo);
throw resException;
}
[edit]
Using variables
- Local variables (Variables used temporarily within a method)
- Use meaningful/descriptive names for local variables
- Use meaningful/descriptive names for local variables
Examples: Instead of long t1, use long startTime Instead of RRDTypeServiceEngine s, use RRDTypeServiceEngine rrdTypeServiceEngine or RRDTypeServiceEngine rrdServiceEngine
- Member Variables (Variables part of the class definition)
- Using a different naming scheme for member variables has been thought about but not decided upon. A different naming scheme (example, append ps_ before the usual names as in ps_rrdTypeServiceEngine) allows easy identification of member variables in methods. Usage of this scheme is currently being evaluated.
[edit]
Testing
- Tests should be constructed for features as they are added.
- Compilation errors
- When committing code into the trunk branch of SVN, it is necessary to ensure that it does not cause any compilation errors when the entire project is built
- To Ensure the above, the project has to be first built locally and when no errors are reported, then only it should be committed
- Warnings
- The committed code should not create any warnings
- The code has to be built locally to check if any warnings are created before committing onto the trunk of SVN
- Run time Errors
- When committing a component as ready for run-time tests, test harnesses should be provided
- Test Harnesses should be written, used and committed to the source code repository
- The interfaces defined for the component should be completely tested using these test harnesses and be seen as error free
- A bad test harness is worse than no test harness
- For a component which has different cases of working, the test harness should have the capability to test each possible case
- If a particular case hasn't been tested, the group should be notified about this
- While committing test harnesses, results of the test harness, testing conditions should be described as adequately as possible
- Treat test harnesses on par with other classes/components. If it might be of some need, document aptly
- Define targets in ant for running tester classes (ant/test-run-targets.xml)
[edit]
Dealing with packages
- Packages should be created as per design
- If any new package needs to be created later on, the group should be notified via email
- New packages can be created either after the group has agreed to it or if no response was received in 2 days time
[edit]
Code Reviews
- How often? Who?
[edit]
Sample codes
Sample codes which can be seen as guidelines are available.
Sample 1 Coding_Standard_Samples_1
