Coding Convention: Difference between revisions
mNo edit summary |
No edit summary |
||
| (51 intermediate revisions by 2 users not shown) | |||
| Line 1: | Line 1: | ||
In this document I have gathered a set of very simple coding conventions. These apply to all ''org.simantics'' projects and are recommended for anyone developing with Java. I try to address some of the common mistakes that recur in a multi-developer project. A vast number of bugs can be back-tracked to failure of common understanding, a situation of mis-matching assumptions. | |||
It is natural that our handiwork changes as we grow in experience. Sometimes a developer makes code where the rules or assumptions have changed along the way. For co-developers and future selves it can be a confusing trail. For the reader inconsistent convention leaves room for speculation, and can drive towards reaction-style-coding: guessing and fixing-on-error. Bad quality has a bad manner of propagating forwards, its common in projects with no rules at all. I'll get into more concrete specifics. | |||
By following a common set of rules, hours are saved in avoided debugging. Quality is increased as developer minds can focus on the actual issues. | |||
==Java Conventions== | |||
Pivotal documents for new java developers: | |||
* [http://www.oracle.com/technetwork/java/codeconv-138413.html Java Coding Style] | |||
* [http://www.oracle.com/technetwork/java/index-137868.html Java Comments Quick Reference] | |||
==Argument Assumption== | ==Argument Assumption== | ||
| Line 73: | Line 84: | ||
==Trust your assumptions== | ==Trust your assumptions== | ||
*'''You have a code of conduct - give it a chance.''' <br/> | *'''You have a code of conduct - give it a chance.''' <br/> | ||
The callee can trust the caller. | The callee can trust the caller. Remember VM will do null checkings anyway. | ||
<div style="background: #f3fff3;"> | <div style="background: #f3fff3;"> | ||
<syntaxhighlight lang="java"> | <syntaxhighlight lang="java"> | ||
BigInteger multiply(BigInteger a, BigInteger b) throw IOException { | |||
return a.multiply(b); | |||
} | } | ||
</syntaxhighlight> | </syntaxhighlight> | ||
| Line 88: | Line 97: | ||
<div style="background: #f3fff3;"> | <div style="background: #f3fff3;"> | ||
<syntaxhighlight lang="java"> | <syntaxhighlight lang="java"> | ||
System.out.println( | System.out.println( multiply(a, b) ); | ||
</syntaxhighlight> | </syntaxhighlight> | ||
<br/> | <br/> | ||
| Line 94: | Line 103: | ||
<br/> | <br/> | ||
In most cases there is no good reason to do redundant checking, especially at run-time. | |||
<div style="background: #fff3f3;"> | <div style="background: #fff3f3;"> | ||
<syntaxhighlight lang="java"> | <syntaxhighlight lang="java"> | ||
BigInteger multiply(BigInteger a, BigInteger b) throw IOException { | |||
if ( | if ( a == null || b == null ) throw IllegalArgumentException("Non-null argument is expected"); | ||
return a.multiply(b); | |||
} | } | ||
</syntaxhighlight> | </syntaxhighlight> | ||
| Line 109: | Line 116: | ||
<div style="background: #fff3f3;"> | <div style="background: #fff3f3;"> | ||
<syntaxhighlight lang="java"> | <syntaxhighlight lang="java"> | ||
Object x = | Object x = multiply( a, b ); | ||
if ( x != null ) System.out.println( x ); | if ( x != null ) System.out.println( x ); | ||
</syntaxhighlight> | </syntaxhighlight> | ||
| Line 116: | Line 123: | ||
<br/> | <br/> | ||
Use assertions if you must. | Use assertions if you must. Checking does improve quality a bit and helps in early detection of problems, but is seldom needed if convention is followed. Assertion is not considered as run-time checking since they can be disabled from the VM. | ||
<div style="background: #ffffea;"> | <div style="background: #ffffea;"> | ||
<syntaxhighlight lang="java" style="background: #dfd;"> | <syntaxhighlight lang="java" style="background: #dfd;"> | ||
BigInteger multiply(BigInteger a, BigInteger b) { | |||
assert( a != null && b != null ); | |||
return a.multiply(b); | |||
} | |||
</syntaxhighlight> | |||
<br/> | |||
</div> | |||
==Method migration== | |||
This rule applies to code that is published and in wide use. | |||
*'''API doesn't change between minor releases.''' | |||
In case of faulty design, old methods singature are preserved within the major version and marked '''@Deprecated'''. They can be removed in the next major release. | |||
<div style="background: #fff3f3;"> | |||
<syntaxhighlight lang="java"> | |||
@Deprecated | |||
Object getValue(Object newValue); | |||
</syntaxhighlight> | |||
<br/> | |||
</div> | |||
*'''Documentation is correct, the implementation is faulty.''' | |||
In situation where there is a mismatch between the documentation and the implementation, the documentation prevails and the fault is in the implementation. | |||
In this example there is problematic method that may return an unexpected null. | |||
<div style="background: #fff3f3;"> | |||
<syntaxhighlight lang="java"> | |||
/** | |||
* Deserialize an object from an input stream. | |||
* | |||
* @param is source stream | |||
* @return the object | |||
**/ | |||
Object deserialize(InputStream is) { | Object deserialize(InputStream is) { | ||
try { | |||
int x = is.read(); | |||
... | |||
return result; | |||
} catch (IOException e) { | |||
return null; | |||
} | |||
} | |||
</syntaxhighlight> | |||
<br/> | |||
</div> | |||
The assumptions that can be derived from the '''documentation''' are left unchanged and the '''implementation''' is corrected. | |||
<div style="background: #f3fff3;"> | |||
<syntaxhighlight lang="java"> | |||
/** | |||
* Deserialize an object from an input stream. | |||
* | |||
* @deprecated use deserialize2, it has better error control | |||
* @param is source stream | |||
* @return the object | |||
* @throws RuntimeIOException in case of IO problems | |||
**/ | |||
Object deserialize(InputStream is) throws RuntimeIOException | |||
{ | |||
try { | |||
int x = is.read(); | |||
... | |||
return result; | |||
} catch (IOException e) { | |||
throw new RuntimeIOException( e ); | |||
} | |||
} | |||
</syntaxhighlight> | |||
<br/> | |||
</div> | |||
The faulty method can be replaced with correct one in the next major version release. | |||
<div style="background: #f3fff3;"> | |||
<syntaxhighlight lang="java"> | |||
/** | |||
* Deserialize an object from an input stream. | |||
* | |||
* @param is source stream | |||
* @return an object | |||
* @throws IOException in case of problems | |||
**/ | |||
Object deserialize(InputStream is) throws IOException | |||
{ | |||
int x = is.read(); | |||
... | ... | ||
return result; | return result; | ||
} | } | ||
</syntaxhighlight> | </syntaxhighlight> | ||
<br/> | <br/> | ||
</div> | </div> | ||
--<br/> | |||
[[User:Toni Kalajainen|Toni Kalajainen]] | |||
[[Category: Miscellaneous Documents]] | |||
Latest revision as of 15:51, 28 January 2011
In this document I have gathered a set of very simple coding conventions. These apply to all org.simantics projects and are recommended for anyone developing with Java. I try to address some of the common mistakes that recur in a multi-developer project. A vast number of bugs can be back-tracked to failure of common understanding, a situation of mis-matching assumptions.
It is natural that our handiwork changes as we grow in experience. Sometimes a developer makes code where the rules or assumptions have changed along the way. For co-developers and future selves it can be a confusing trail. For the reader inconsistent convention leaves room for speculation, and can drive towards reaction-style-coding: guessing and fixing-on-error. Bad quality has a bad manner of propagating forwards, its common in projects with no rules at all. I'll get into more concrete specifics.
By following a common set of rules, hours are saved in avoided debugging. Quality is increased as developer minds can focus on the actual issues.
Java Conventions
Pivotal documents for new java developers:
Argument Assumption
- All method arguments are non-null unless explicitely stated otherwise in documentation.
The default assumption is that an argument is non-null. This applies to undocumented methods too.
<syntaxhighlight lang="java">
/**
* Read the object from a file.
*
* @param file
*/
void read(File file);
// and void read(File file);
</syntaxhighlight>
A null possibility must be explicitely stated.
<syntaxhighlight lang="java">
/**
* Write or remove existing value.
*
* @param newValue new value or null</t> to remove the existing value
*/
void setValue(Object newValue);
</syntaxhighlight>
Return value assumption
- All return values are non-null unless explicitely stated otherwise in documentation.
The thumb rule is that the return value is non-null. It applies to undocumented methods aswell.
<syntaxhighlight lang="java">
/**
* Get the value
*
* @return the value
*/
Object get();
// and Object get();
</syntaxhighlight>
Null option as return value is always explicitely documented.
<syntaxhighlight lang="java">
/**
* Get possibly existing value
*
* @return the value is exists, otherwise null
*/
Object get();
</syntaxhighlight>
Trust your assumptions
- You have a code of conduct - give it a chance.
The callee can trust the caller. Remember VM will do null checkings anyway.
<syntaxhighlight lang="java">
BigInteger multiply(BigInteger a, BigInteger b) throw IOException {
return a.multiply(b);
}
</syntaxhighlight>
And the caller the callee.
<syntaxhighlight lang="java">
System.out.println( multiply(a, b) );
</syntaxhighlight>
In most cases there is no good reason to do redundant checking, especially at run-time.
<syntaxhighlight lang="java">
BigInteger multiply(BigInteger a, BigInteger b) throw IOException {
if ( a == null || b == null ) throw IllegalArgumentException("Non-null argument is expected");
return a.multiply(b);
}
</syntaxhighlight>
Nor caller.
<syntaxhighlight lang="java">
Object x = multiply( a, b ); if ( x != null ) System.out.println( x );
</syntaxhighlight>
Use assertions if you must. Checking does improve quality a bit and helps in early detection of problems, but is seldom needed if convention is followed. Assertion is not considered as run-time checking since they can be disabled from the VM.
<syntaxhighlight lang="java" style="background: #dfd;">
BigInteger multiply(BigInteger a, BigInteger b) {
assert( a != null && b != null );
return a.multiply(b);
}
</syntaxhighlight>
Method migration
This rule applies to code that is published and in wide use.
- API doesn't change between minor releases.
In case of faulty design, old methods singature are preserved within the major version and marked @Deprecated. They can be removed in the next major release.
<syntaxhighlight lang="java">
@Deprecated Object getValue(Object newValue);
</syntaxhighlight>
- Documentation is correct, the implementation is faulty.
In situation where there is a mismatch between the documentation and the implementation, the documentation prevails and the fault is in the implementation.
In this example there is problematic method that may return an unexpected null.
<syntaxhighlight lang="java">
/**
* Deserialize an object from an input stream.
*
* @param is source stream
* @return the object
**/
Object deserialize(InputStream is) {
try {
int x = is.read();
...
return result;
} catch (IOException e) {
return null;
}
}
</syntaxhighlight>
The assumptions that can be derived from the documentation are left unchanged and the implementation is corrected.
<syntaxhighlight lang="java">
/**
* Deserialize an object from an input stream.
*
* @deprecated use deserialize2, it has better error control
* @param is source stream
* @return the object
* @throws RuntimeIOException in case of IO problems
**/
Object deserialize(InputStream is) throws RuntimeIOException
{
try {
int x = is.read();
...
return result;
} catch (IOException e) {
throw new RuntimeIOException( e );
}
}
</syntaxhighlight>
The faulty method can be replaced with correct one in the next major version release.
<syntaxhighlight lang="java">
/**
* Deserialize an object from an input stream.
*
* @param is source stream
* @return an object
* @throws IOException in case of problems
**/
Object deserialize(InputStream is) throws IOException
{
int x = is.read();
...
return result;
}
</syntaxhighlight>