This page describes the rules that we use when writing source-code.
In addition to the license header, all source code files have to bear the following copyright header:
/* * © /Vorname und Name des Studenten/, Freie Universität Berlin /Jahr/ */
ext-src de.fu_berlin.inf.#project# for instance de.fu_berlin.inf.wikidoc or de.fu_berlin.inf.ecg. This follows Suns suggestions of how to choose a package name.
readme.txt which contains instructions for how to run the application and gives a short overview of the code-base.
I IProject, IPreferenceStore
Listener to it MouseListener
Adapter instead of Listener to it MouseAdapter
events, relative to their corresponding class Foo resides in package de.fu_berlin.inf.bar, then FooListener and FooAdapter have to reside in de.fu_berlin.inf.bar.events
protected or public to indicate whether this attribute is part of the public signature (then public) or not (then protected).
private) you need to explain the reason for this in the JavaDoc comment, because a private attribute should make anybody stop and think "Uh-Oh. I better don't mess with this.". If we use private for everything then we would not need protected at all. protected instead) as it restricts the possibilities of others.
protected, or define an interface)
public class A implements B, C, D {
…
you should write
public class A implements D {
B b = new B(){
…
};
C c = new C(){
};
…
public void … {
…
if (condition){
// long block of code
}
}
you should write
public void … {
…
if (!condition)
return
// long block of code
}
/**
* Get positions of slashes in the filename.
* @param filename may be null
* @return Null-based indices into the string pointing to the slash positions.
*/
public int[] findAllSlashes(String filename) {
if (filename == null)
return new int[];
…
@ui or @swing or @swt - This method needs to be called from the UI thread, otherwise an exception might occur.
@nonBlocking - This method does return before finishing its activities. If there is at least one method is a class which is non-blocking it is highly recommended to put @blocking to all other methods to help users recognize that blocking behavior is important for this class
@blocking - This method is potentially long-running and blocks until it is done. This is the default for all method, which are unmarked.
@valueObject - The objects of this class are immutable. All methods are side effect free.
@nonReentrant - This method cannot be called twice at the same time.
@threadsafe - This method can safely be called twice at the same time.
@caching - If possible, this method uses a cache to calculate the return value.
Whenever a method is long-running, i.e. there is a chance that it will take longer than 100ms or involves any external factors such as the user or input/output, the software engineer is responsible to provide a way to track progress of the operation and provide to the caller the possibility to cancel the operation.
If the software engineer does not provide such opportunity the user experience might be reduced. For instance in Saros, there used to be no possibility to cancel a file transfer from one user to the other but just to cancel in between the files. This behavior seems okay, but once we started to see archive files of 10 MB and more, which could only be canceled by disconnecting from the Jabber-Server, the undesireability of the behavior becomes pretty clear.
Fortunately enough there is a straight-forward and simple solution, which also improves the general threading behavior of the application: The class to use is calledSubMonitor which implements the IProgressMonitor interface.
Now all methods which are long running, need to be changed to take a SubMonitor as a last parameter (this is our convention):
public Result computeSomething(List<Something> input, …, SubMonitor progress){
Inside those methods, first, we need to initialize the ProgressMonitor with the name of the task and the number of steps this task will take (if the number of steps is unknown we set it to a large integer, 10000 is our convention):
progress.beginTask("Computing Something", input.size());
Now whenever we have made some progress towards this task, we can report this to the monitor:
for (Something some : input){
… process input
progress.worked(1)
}
At the end of the task, we should report, that we are done with the task:
progress.done()This basic contract of
beginTask(), worked(), and done() is sufficient to achieve the basic uses of progress monitoring.
public void computeInTwoSteps(SubMonitor progress){
progress.beginTask("Compute in two steps", 2);
computeFirstStep(progress.newChild(1));
computeSecondStep(progress.newChild(1));
progress.done();
}
This code will pass two =SubMonitor=s to the two methods, which then are free to use them just as the parent method did:
public void computeFirstStep(SubMonitor progress){
progress.beginTask("Compute the first step", 140);
…
progress.worked(5); // etc.
…
progress.done();
}
A progress monitor provides 3 ways to report information from a long running operation to the user
worked()
beginTask(String) and setTaskName(String)
subTask(String)
This information is typically presented to the user as a Dialog with a message being equal to the taskname of the top level progress monitor, a progress bar showing the growing amount of work already done and a label for the current sub-task which switches everytime the sub-task is being set.
Since the taskName is fixed (by default), only the top level task name is shown to the user. The task name of the nested operations are never shown to the user. To report status messages from nested operations, the sub-task needs to be used:
public void computeInTwoSteps(SubMonitor progress){
progress.beginTask("Compute in two steps", 2);
progress.subTask("Two Step Computation: Step 1");
computeFirstStep(progress.newChild(1));
progress.subTask("Two Step Computation: Step 2");
computeSecondStep(progress.newChild(1));
progress.done();
}
To have a progress dialog on operations for which the amount of steps are unknown, the following solution is recommended:
while (!done()){
… do work
progress.setWorkRemaining(1000);
progress.worked(1);
}
This code will advance the progress bar 0,1% of the remaining total of the progress monitor and thus logarithmically approach 100% worked. The percentage 0,1% should be adjusted to achieve 50% progress on the expected number of work steps.
for (Something some : input){
if (progress.isCanceled())
return;
… process input
progress.worked(1)
}
The easiest way to response to a request for cancellation is to just return as shown above, but in most cases this is undesirable, because the caller will not know whether the operation finished or not. Instead, methods should rather throw a CancellationException so that a caller can recognize that the operation was canceled:
/**
* @cancelable This long-running can be canceled via the given
* progress monitor and will throw an CancellationException
* in this case.
*/
public BigInteger factorial(int n, SubMonitor progress){
progress.beginTask("Computing Factorial of " + n, n);
BigInteger result = BigInteger.ONE;
for (int i = 1; i < n; i++){
if (progress.isCanceled())
throw new CancellationException();
result = result.multiply(BigInteger.valueOf(i));
progress.worked(1);
}
progress.done();
return result;
}
Btw: It is an convention that we try to avoid InterruptedException for this, because it is a checked exception and thus cumbersome for the caller. To maintain this convention, a method MUST specify whether it is cancelable, by providing the demonstrated JavaDoc tag.
container.addComponent(A.class)
container.getComponent(A.class)
@Inject) and AFTER the constructor has been called will fill those fields with the dependencies. This type of injection, provides the code with the best readability (constructors with many parameters are difficult to read) but is more difficult to manage, because dependencies are not available in the constructor.
To avoid repeated code blocks in an Observable like
class Observable {
List<Listener> listeners = new …
public void addListener(Listener listener){listeners.add(listener)}
public void removeListener(Listener listener){...}
public void someMethod() {
…
// notify all listeners
for (Listener l : listeners) {
l.notify();
}
}
public void someMethod2() {
…
// notify all listeners again
for (Listener l : listeners) {
l.notify();
}
}
}
It is recommended to use a helper class BroadcastListener that
provides a method to notify all its registered listeners. The
BroadcastListener should be a singleton managed by PicoContainer.
public class BroadcastListener implements Listener {
List<Listener> listeners = new …
public void add(Listener listener){listeners.add(listener)}
public void remove(Listener listener){...}
public void notify() {
for (Listener l : listeners) {
l.notify();
}
}
}
The code for an Observable becomes therfore much simpler. It only needs
to know the BroadcastListener and can easily notify all registered
listeners at once.
class Observable {
BroadcastListener listener = new BroadcastListener();
public void someMethod(){
…
listener.notify();
}
public void someMethod2(){
…
listener.notify();
}
}
public void setSharedProject(SharedProject newSharedProject) {
if (sharedProject == newSharedProject)
return;
// Unregister from old project
if (sharedProject != null) {
sharedProject.removeListener(this);
}
sharedProject = newSharedProject;
// Register to new project
if (sharedProject != null) {
sharedProject.addListener(this);
}
}
All source documents are expected to be documented and written in English.
/*
* one comment that takes up at least
* two lines
*/
There are six categories of comments:
A counter-example:
/**
* return a stack of String,
* @param text
* @return Stack
*/
public Stack getFormatJavadocLine(String text) {
StringTokenizer st = new StringTokenizer(text, "\n");
Stack stack = new Stack();
while (st.hasMoreTokens()) {
stack.add(st.nextToken());
}
return stack;
}
The documentation is absolutely useless as it just repeats the signature and even fails to explain the method name. Furthermore the method name is wrong for the task that is actually performed. The important question whether this method returns a stack of the individual lines in text from top to bottom or bottom to top remains unanswered.
/*
* TODO FIX: Our caller should be able to distinguish whether the
* query failed or it is an IM client which sends back the message
*/
/*
* move all chess pieces to start position
*/
/*
* initialize a new chess game
*/
Acceptable code comments are summary comments, intent comments and information that cannot be expressed in code. Markers are acceptable during development but should be removed before release. Try to avoid comments that repeat or explain the code.
[NOP] This commit did not have any effect and only affects whitespace, removing unused methods, fixing documentation typos, etc.
[DOCU] Improves JavaDocs or comments
[TASK] Adds things that need to be done
[API] Affects the interface or dependencies of a component without creating any new functionality or fixing an existing bug
[INTERNAL] Only affects the details of the implementation without any effects to users of the component.
[REFACTOR] API or INTERNAL changes, which are done using automated tool support. So while REFACTOR changes usually result in large diffs, they are not very error prone. Caution: A refactoring should always be a separate patch, because refactorings are very hard to read.
[FIX] #Number: Bug description from SF Fixes a bug, if possible attach the SourceForge Bug #ID
[FEATURE] Improves the functionality of the software
[LOG] Improves Logging with Log4j
[UI] Improvements to the user interface
[auto-props]
*.java = svn:eol-style=LF
*.txt = svn:eol-style=LF
We expect that all source code used in thesis to deal gracefully with errors. The goals to aim for should be:
RuntimeException that might have slipped up.
Use the following method for this (you might want to pass up =RuntimeException=s up the stack as well):
/**
* Return a new Runnable which runs the given runnable but catches all
* RuntimeExceptions and logs them to the given logger.
*
* Errors are logged and rethrown.
*
* This method does NOT actually run the given runnable, but only wraps it.
*/
public static Runnable wrapSafe(final Logger log, final Runnable runnable) {
return new Runnable() {
public void run() {
try {
runnable.run();
} catch (RuntimeException e) {
log.error("Internal Error:", e);
} catch (Error e) {
log.error("Internal Fatal Error:", e);
// Rethrow errors (such as an OutOfMemoryError)
throw e;
}
}
};
}
When developing in Eclipse the following code-snippets might help:
Display.getDefault().syncExec(new Runnable() {
public void run() {
MessageDialog.openError(Display.getDefault().getActiveShell(),
"Dialog Title", "Error message");
}
});YourPlugin.getDefault().getLog().log( new Status(IStatus.ERROR, "Plug-In ID goes here", IStatus.ERROR, message, e));
Anti-example:
public Javadoc updateJavadoc(String filename, String name,
String newJavadocText, int isType) {
Javadoc jd = null;
try {
… Try to update Javadoc …
} catch (Exception e) { // No, no, no!
e.printStackTrace();
}
System.out.println("The new javadoc-------\n" + jd);
return jd;
}
public Javadoc updateJavadoc(String filename, String name,
String newJavadocText, int isType) {
Javadoc jd = null;
try {
… Try to update Javadoc …
} catch (IOException e){ // Catch the checked exceptions that occur
// Transform to runtime exception if you don't know what to do with it.
throw new RuntimeException(e);
} // let all runtime exceptions go up the stack
System.out.println("The new javadoc-------\n" + jd);
return jd;
}
How to do it right:
public Javadoc updateJavadoc(String filename, String name,
String newJavadocText, int isType) throws IOException{
Javadoc jd = null;
try {
… Try to update Javadoc …
} catch (IOException e){ // Catch the checked exceptions that occur
// bring the internal logic back to a stable state (if you can)
throw e; // let the caller handle this exception
}
System.out.println("The new javadoc-------\n" + jd);
return jd;
}
private static final Logger log = Logger.getLogger(${enclosing_type}.class);
ERROR An error should be printed if something occurred which is the fault of the developer and which will cause unexpected behavior for the user.
WARN A warning should be printed if something occurred which is the fault of the developer but which while not expected to occur should be handled gracefully by the application.
INFO Any information that is of interest to the user may be printed using INFO.
DEBUG Any information which might be useful for the developer when figuring out what the application did may be printed as DEBUG. The amount of information printed should not cause the performance of the application to suffer (use TRACE for this).
TRACE Detailed information about the program execution should use the TRACE level. This information usually is so detailed that the application runs slower and thus should never be enabled in a production version.
try {
Thread.sleep(500);
} catch(InterruptedException e){
Thread.currentThread().interrupt(); // The code will continue after after the catch
}
boolean interrupted = false;
try {
while (looping){
// do stuff
try {
Thread.sleep(500);
} catch(InterruptedException e){
interrupted = true;
}
}
} finally {
if (interrupted)
Thread.currentThread().interrupt(); // The code will continue after after the catch
}
/** * @nonInterruptable This method does not support being interrupted */
IOUtils.closeQuitely() for closing streams, as it handles all cases (null, exceptions) in one line
if (...){
return null;
} else {
// do a lot of things…
// nested very…
// …deeply
}
can easily become
if (...){
return null;
}
// do a lot of things…
// nested only…
// …deeply now (you saved a level of nesting)
-ea and use asserts to let people understand what you were assuming in your code.
IPath to a String use IPath.toPortableString() to convert from a String to an IPath use Path.fromPortableString() (one could also use toString() and new Path(String), but we prefer the from/toPortable version).
enum instead of a bunch of static final int fields. Then you have type safety for related constants and more readable debugging output is possible because you get meaningful names instead of "magic" numbers.