Difference between revisions of "Coding Convention"
m |
|||
(56 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== | ||
*'''All method arguments are non-null unless explicitely stated otherwise in documentation. ''' | *'''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. | ||
<div style="background: #f3fff3;"> | <div style="background: #f3fff3;"> | ||
<syntaxhighlight lang="java"> | <syntaxhighlight lang="java"> | ||
Line 13: | Line 25: | ||
void read(File file); | void read(File file); | ||
+ | // and | ||
+ | void read(File file); | ||
+ | </syntaxhighlight> | ||
+ | <br/> | ||
+ | </div> | ||
+ | <br/> | ||
+ | |||
+ | A ''null'' possibility must be explicitely stated. | ||
+ | <div style="background: #f3fff3;"> | ||
+ | <syntaxhighlight lang="java"> | ||
/** | /** | ||
* Write or remove existing value. | * Write or remove existing value. | ||
Line 23: | Line 45: | ||
</div> | </div> | ||
<br/> | <br/> | ||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
==Return value assumption== | ==Return value assumption== | ||
− | *'''All return values are non-null unless explicitely stated otherwise in documentation.''' | + | *'''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. | ||
<div style="background: #f3fff3;"> | <div style="background: #f3fff3;"> | ||
<syntaxhighlight lang="java"> | <syntaxhighlight lang="java"> | ||
Line 44: | Line 58: | ||
* @return the value | * @return the value | ||
*/ | */ | ||
− | Object get() | + | Object get(); |
− | / | + | // and |
− | |||
− | |||
− | |||
− | |||
Object get(); | Object get(); | ||
</syntaxhighlight> | </syntaxhighlight> | ||
Line 57: | Line 67: | ||
<br/> | <br/> | ||
− | + | ''Null'' option as return value is always explicitely documented. | |
<div style="background: #f3fff3;"> | <div style="background: #f3fff3;"> | ||
<syntaxhighlight lang="java"> | <syntaxhighlight lang="java"> | ||
+ | /** | ||
+ | * Get possibly existing value | ||
+ | * | ||
+ | * @return the value is exists, otherwise <tt>null</tt> | ||
+ | */ | ||
Object get(); | Object get(); | ||
</syntaxhighlight> | </syntaxhighlight> | ||
<br/> | <br/> | ||
</div> | </div> | ||
− | + | <br/> | |
==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 84: | 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/> | ||
</div> | </div> | ||
− | + | <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 103: | 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> | ||
<br/> | <br/> | ||
</div> | </div> | ||
+ | <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.
Contents
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>