Coding Convention

From Developer Documents
Jump to: navigation, search

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.

 
    /**
      * Read the object from a file.
      *
      * @param file 
      */
    void read(File file);
 
    // and
    void read(File file);



A null possibility must be explicitely stated.

 
    /**
      * Write or remove existing value.
      *
      * @param newValue new value or <tt>null</t> to remove the existing value
      */
    void setValue(Object newValue);




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.

 
    /**
      * Get the value
      *
      * @return the value
      */
    Object get();
 
    // and
    Object get();



Null option as return value is always explicitely documented.

 
    /**
      * Get possibly existing value
      *
      * @return the value is exists, otherwise <tt>null</tt>
      */
    Object get();




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.

 
    BigInteger multiply(BigInteger a, BigInteger b) throw IOException {
        return a.multiply(b);
    }


And the caller the callee.

 
    System.out.println( multiply(a, b) );



In most cases there is no good reason to do redundant checking, especially at run-time.

 
    BigInteger multiply(BigInteger a, BigInteger b) throw IOException {
        if ( a == null || b == null ) throw IllegalArgumentException("Non-null argument is expected");
        return a.multiply(b);
    }


Nor caller.

 
    Object x = multiply( a, b );
    if ( x != null ) System.out.println( x );



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.

 
    BigInteger multiply(BigInteger a, BigInteger b) {
        assert( a != null && b != null );
        return a.multiply(b);
    }



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.

 
    @Deprecated
    Object getValue(Object newValue);


  • 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.

 
    /**
     * 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;
        }
    }


The assumptions that can be derived from the documentation are left unchanged and the implementation is corrected.

 
    /**
     * 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 );
        }
    }


The faulty method can be replaced with correct one in the next major version release.

 
    /**
     * 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;
    }



--
Toni Kalajainen