[DODS HEAD] Code quality

I came across this method today and would like to propose several code
improvements to DODS.  The code is in the file GenericDO but this applies to
almost all the files.

    /**
     * Updates the contents of this object in the database
     * but only if the datab object is dirty.  This requires
     * that all set methods in the data objects keep track
     * if a value has changed.
     */
    public synchronized void executeInsert(DBConnection conn)
            throws SQLException, DBRowUpdateException {
        /*        if (!updateInProgress) {*/
            if ( ! notUsingOId )
    // v. strahinja, 27 sep 2002            printMsg( Logger.DEBUG2,
                printMsg( Level.DEBUG,
                getClass().getName() + ".executeInsert():" + persistent + ": "
                + get_OId().toString()
                + ": " + get_Version() + ": " + get_NewVersion() );
            super.executeInsert(conn);
            /*            updateInProgress = !getAutoSave();
            } */
    }

Here are my remarks:

1) keeping old code as commented out is very misleading and really not easy
to learn.  To see old code, just do a diff between two versions in cvs.

2) do not mix spaces and tabs.  Depending on the user's IDE, the code will
look quite different.

For example, my IDE (Eclipse) displays the code as follow (tabs replaced):

    /*        if (!updateInProgress) {*/
        if ( ! notUsingOId )
    // v. strahinja, 27 sep 2002            printMsg( Logger.DEBUG2,
                printMsg( Level.DEBUG,
        getClass().getName() + ".executeInsert():" + persistent + ": "
        + get_OId().toString()
        + ": " + get_Version() + ": " + get_NewVersion() );
            super.executeInsert(conn);
        /*            updateInProgress = !getAutoSave();
        } */

so I don't know if the super.executeInsert should only be invoked when
notUsingOId is false and in that case it's a bug (because only a message is
logged in that case) or if everything is fine.

3) use braces with if statements.  Even for a single line of code.

4) use

    if (log.isDebugEnable()) {
        log.debug(...);
    }

   when logging.  When using a helper method (like currently done) the
message is created even if the it will not be logged (because the debug level
is not enabled)

Solutions to some of these these "problems"

2) a good code formatting tool would easily fix this.
3) use PMD (pmd.sf.net) to check for braces (and other code review)

Best regards,

-Vladimir

PS: don't start to flame, please.  I want to be constructive but you first
have to point out the problems before fixing them.

=====
Vladimir Ritz Bossicard
www.bossicard.com

__________________________________
Do you Yahoo!?
The New Yahoo! Shopping - with improved product search
http://shopping.yahoo.com