Difference between revisions of "Coding Convention"

From Developer Documents
Jump to navigation Jump to search
m
 
(63 intermediate revisions by 2 users not shown)
Line 1: Line 1:
Simantics coding conventions are gathered in this document.  
+
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/>
 
Non-null assumption applies in undocumented methods aswell. This method mustn't be called with a null argument.
 
<div style="background: #f3fff3;">
 
<syntaxhighlight lang="java">   
 
    void read(InputStream is);
 
</syntaxhighlight>
 
<br/>
 
</div>
 
 
  
  
 
==Return value assumption==
 
==Return value assumption==
*'''All return values are non-null unless explicitely stated otherwise in documentation.'''<br/>
+
*'''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() throws ValueUnavailableException;
+
     Object get();
  
     /**
+
     // and
      * Get possibly existing value
 
      *
 
      * @return the value is exists, otherwise <tt>null</tt>
 
      */
 
 
     Object get();
 
     Object get();
 
</syntaxhighlight>
 
</syntaxhighlight>
Line 57: Line 67:
 
<br/>
 
<br/>
  
Non-null assumption applies in undocumented methods too, there is always a non-null return value.
+
''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">     
     void read(InputStream is) {
+
     BigInteger multiply(BigInteger a, BigInteger b) throw IOException {
         int  x = is.read();
+
         return a.multiply(b);
 
     }
 
     }
 
</syntaxhighlight>
 
</syntaxhighlight>
Line 82: Line 97:
 
<div style="background: #f3fff3;">
 
<div style="background: #f3fff3;">
 
<syntaxhighlight lang="java">     
 
<syntaxhighlight lang="java">     
     System.out.println( serialiser.deserialize( is ) ); <br/>
+
     System.out.println( multiply(a, b) );
 
</syntaxhighlight>
 
</syntaxhighlight>
 
<br/>
 
<br/>
 
</div>
 
</div>
There is no need to do redundant checking, especially at run-time.  
+
<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">     
     void read(InputStream is) {
+
     BigInteger multiply(BigInteger a, BigInteger b) throw IOException {
         if ( is == null ) throw IllegalArgumentException("Non-null argument");
+
         if ( a == null || b == null ) throw IllegalArgumentException("Non-null argument is expected");
         int  x = is.read();
+
         return a.multiply(b);
 
     }  
 
     }  
 
</syntaxhighlight>
 
</syntaxhighlight>
Line 99: Line 116:
 
<div style="background: #fff3f3;">
 
<div style="background: #fff3f3;">
 
<syntaxhighlight lang="java">     
 
<syntaxhighlight lang="java">     
     Object x = serializer.deserialize( is );
+
     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. It sometimes improve quality and debuggability.
+
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: #fffff3;">
+
<div style="background: #ffffea;">
 
<syntaxhighlight lang="java" style="background: #dfd;">     
 
<syntaxhighlight lang="java" style="background: #dfd;">     
     void read(InputStream is) {
+
     BigInteger multiply(BigInteger a, BigInteger b) {
         assert( is != null );
+
         assert( a != null && b != null );
         int x = is.read();
+
         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) {
 +
        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>
 
</syntaxhighlight>
 
<br/>
 
<br/>
 
</div>
 
</div>
  
==Return value assumption==
+
The faulty method can be replaced with correct one in the next major version release.
All non-void methods return a non-null value unless expilitely stated otherwise in javadoc.
+
<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;
 +
    }
 +
 
 +
</syntaxhighlight>
 +
<br/>
 +
</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>


--
Toni Kalajainen