Dashboard > uPortal > Code Best Practices
uPortal Log In   View a printable version of the current page.
Code Best Practices

Added by Brad Johnson , last edited by Mark Mazelin on Apr 28, 2006  (view change)
Labels: 
(None)

File Encoding

Text file encoding should be ISO-8859-1 for Java files.
Eclipse users go to: Window > Preferences > Workbench > Editors Then set: Text file encoding to ISO-8859-1
(Eclipse 3.1 users need to go to: Window > Preferences > General > Editors)

Coding formatting

Our coding conventions

We adopt the Code Conventions for the JavaTM Programming Language as our baseline coding conventions. The linked document provides conventions for such things as brace placement, naming of STATIC_FIELDS, and so forth. See also Developer Conventions for further specifications of things not made clear in the baseline code conventions.

When to format

Do not beautify code just for the sake of beautifying code. Fix formatting problems. Improve the formatting of the localized code that you're re-writing. And save everyone having to make decisions about the cost of change vs. the value of better formatting by having the best formatting you can have from when your code is first introduced.

No Tabs

Do not use Tabs. Instead use an appropriate number of spaces, where 4 spaces is the standard indentation increment. Convince your IDE to use spaces rather than tabs. You can configure Eclipse to insert spaces for tabs, so you can still use the Tab key.

No single line If statements

Please don't use the keyword if without braces indicating the block of code to be conditionally executed. Non-braced if statements are dangerous! The next developer modifying the code often adds another line to the conditional block, expecting it to be in the block. Having brackets makes it easier to read and safer to modify.

One . Operator Per Line

Do not use more dot opperators per line than needed.

Bad.java
lines.ofJavaCode(that.involve(numerousReferences).thatMightBeNull().suchThat().anNpe()).isLessAmbigous();
Good.java
Involve i = that.involve(numerousReferences);
MightBeNull mbn = i.thatMightBeNull();
That t = mbn.suchThat();
Npe n = t.anNpe();
JavaCode jc = lines.ofJavaCode(n)
jc.isLessAmbigous();
Unknown macro: {nocc}

The second example makes following a stack trace, finding a NullPointerException, ClassCastException or any other problem much easier as the line number will point you to the single statement that caused the problem. Putting one statement on each line also makes using an debugger much easier as you can inspect the value of each variable before it is used in the next statement.

Code

  • It is generally not a good idea to declare a method as throws Exception. See Effective Java.
  • Eliminate all compiler warnings.
  • Use a tool like jlint, PMD or FindBugs from time to time.
  • Do Code Reviews early in a project
  • Prepend the uPortal Copyright Header to your code contributions and update it with the present year when you make significant changes.

Database

Logging

Use CommonsLogging in uPortal 2.4 or greater. LogService is deprecated as of uPortal 2.4.
More full discussion of logging best paractices is available.

Log like this:

// This will print the Exception message followed by a full stack trace.
 LOG.error(e,e);

or

// It can be helpful to print out some variables values.
 // The stack trace is still printed as above because the Exception e is not
 // concatenated to the message. Instead the whole exception is passed in.
 LOG.error("username: " + username, e);

Don't log like this, ever:

// No stack trace is printed. Only the text in the error message and the exception
 // class name are printed. This usually isn't enough to track down an Exception.
 LOG.error(e);
 // or this one may even lie to you about the real cause.
 LOG.error("ERROR in MyClass.myMethod() " + e);

This is clearly a case where less is more. Another problem with this is methods get renamed and code gets cut and pasted into a different class. It is very common for these messages to not be in sync with the code. You won't be able to tell where the error is being printed from.

Exception Handling

More full discussion of exception handling best paractices is available.

If a new exception is thrown after catching an old exception be sure to chain them like this:

catch (RuntimeException e) {
    //Creates a new exception, passing in the cause as a constructor argument
    throw new PortalException("Error while performing task", e);
}

If the new exception doesn't take a Throwable argument use the initCause method introduced in JDK1.4

catch (RuntimeException e) {
    //Creates a new exception, setting the cause using the new initCause method from 1.4
    PortalException pe = new PortalException("Error while performing task");
    pe.initCause(e);
    throw pe;
}

Do not ever do the following:

catch (RuntimeException e) {
    //Only includes the message from the cause exception in the new excepion
    //The cause stack trace and line numbers are lost.
    throw new PortalException("Error while performing task: " + e);
}

or worse

catch (RuntimeException e) {
    //No hint to the real cause or where the actual exception occurred is included in the new exception
    throw new PortalException("Error while performing task");
}

Chaining throwables allows the full stack trace and all related error messages to be viewed by the person debugging the system. This is a very important practice, along with always logging throwables correctly, that greatly expedites debugging problems in the portal framework.

IO Streams

  • Streams represent resources which you must always clean up explicitly, by calling the close method. (Eventually you will get an error "too many open files" if you don't clean up after streams.)
  • If multiple streams are chained together, then closing the one which was the last to be constructed, and is thus at the highest level of abstraction, will automatically close all the underlying streams. So, one only has to call close on one stream in order to close (and flush, if applicable) an entire series of related streams.
  • Streams should always be closed in the finally block making sure that close will always get to execute. Following is the code snippet that I suggest. I just use some arbitrary stream classes what you should be looking at how streams should be closed.
//Put this out to make it visible in finally block
        ObjectInput input = null;
            try{
              //use buffering
              InputStream file = new FileInputStream( "employee.ser" );
              InputStream buffer = new BufferedInputStream( file );
              input = new ObjectInputStream ( buffer );
              //deserialize the List
              ArrayList recoveredQuarks = (ArrayList)input.readObject();
              //display its data
              Iterator quarksItr = recoveredQuarks.iterator();
              while ( quarksItr.hasNext() ) {
                System.out.println( (String)quarksItr.next() );
              }
            }catch(IOException ex){
              //Cannot perform input
            }
            catch (ClassNotFoundException ex){
              //Unexpected class found upon input.
            }
            finally{
              try {
                if ( input != null ) {
                  //close "input" and its underlying streams
                  input.close();
                }
              }
              catch (IOException ex){
                //Cannot close input stream.
              }
            }

Use java.sql.PreparedStatement instead of java.sql.Statement

You should use PreparedStatement when substituting parameters into a SQL statement.

If you don't you must escape the string to be inserted to prevent a SQL Injection Attack or SQLException.

Static Final Variable Names

You should use all upper case letters when declare static final variables (Constants). This way Constants will standout in the code. For example declare LOG rather than log as shown below

private static final Log LOG = LogFactory.getLog(someClass.class);
Powered by a free Atlassian Confluence Open Source Project License granted to Java Architectures Special Interest Group. Evaluate Confluence today.
Powered by Atlassian Confluence 2.7.3, the Enterprise Wiki. Bug/feature request - Atlassian news - Contact administrators