The Value in Checking Your Style

See the following code:

package com.example.of.custom.checks;

import com.puppycrawl.tools.checkstyle.api.Check;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
import com.puppycrawl.tools.checkstyle.api.DetailAST;

/**
* Each method that calls a createSession() method should have a corresponding
* close call to ensure that the database connection is closed. It is good practice
* to have the same method that calls the createSession() to call the close()
* method as well as it makes it easier to see that we do not leak open connections.
*
* NOTE: The algorithm for ensuring all opened sessions are closed can probably be improved.
*/
public class SessionClosingCheck extends Check {
    /**
     * Name of the call to the create session method.
     */
    public static final String SESSION_CREATE = "createSession";
    /**
     * Name of the method that closes the session.
     */
    public static final String SESSION_CLOSE = "close";

    /**
     * Context for a method storing the details about calls to createSession and close.
     */
    private SessionClosingContext sessionClosingContext;

    /**
     * For each createSession() we increase the call to the numberOfSessions. For each
     * call to close (as long as it is a session object), we can decrement it.
     * If we reach the end of the file, and we have more than 0 then we have a problem.
     *
     * @return List of tokens that trigger a call to the visitToken method.
     */
    public int[] getDefaultTokens() {
        return new int[] {
            TokenTypes.METHOD_DEF, // Check based on each method declared.
            TokenTypes.IDENT, // for calls to session.close() and createSession()
        };
    }

    /**
     * Handle visiting a method and visiting an IDENT token.
     */
    public void visitToken(DetailAST detailAST) {
        switch (detailAST.getType()) {
            case TokenTypes.METHOD_DEF:
                sessionClosingContext = new SessionClosingContext();
                sessionClosingContext.visitMethodDef(detailAST);
                break;
            case TokenTypes.IDENT:
                // Only check method calls in a method declaration
                if (sessionClosingContext != null) {
                    sessionClosingContext.visitIdent(detailAST);
                }
                break;
        }
    }

    /**
     * When we leave the method declaration, check the context and then log failure of
     * a check if conditions have not been met.
     */
    public void leaveToken(DetailAST detailAST) {
        if (TokenTypes.METHOD_DEF == detailAST.getType()) {
            if (sessionClosingContext != null && !sessionClosingContext.haveAllSessionsBeenClosed()) {
                log(detailAST.getLineNo(), "The count to close() is not more than that of createSession()");
            }
            // Reset the context
            sessionClosingContext = null;
        }
    }

    /**
     * Return the method name given a AST that was found for a MethodDef.
     */
    private String getMethodName(DetailAST methodAST) {
        return methodAST.findFirstToken(TokenTypes.IDENT).getText();
    }

    /**
     * Context class for each method. Within a method definition, we want to keep
     * track of the number of calls to createSession() and then the number of calls
     * to close(). We then compare these at the end of a method call to tell whether
     * or not we have closed all connections we may have opened.
     */
    private class SessionClosingContext {
        // Number of calls found for createSession
        private int sessionCreateCalls = 0;
        // Number of calls found for close
        private int sessionCloseCalls = 0;

        /**
         * Visit the IDENT tokens. In this case we are interested in only the calls
         * to createSession and close.
         */
        public void visitIdent(DetailAST identAST) {
            if (SESSION_CLOSE.equals(identAST.getText())) {
                sessionCloseCalls++;
            } else if (SESSION_CREATE.equals(identAST.getText())) {
                sessionCreateCalls++;
            }
        }

        /**
         * If the createSession is actually a method declaration, we are going to count
         * an additional call to createSession without ignoring it. This method
         * ensures that we don't count these.
         */
        public void visitMethodDef(DetailAST detailAST) {
            if (SESSION_CREATE.equals(getMethodName(detailAST))) {
                sessionCreateCalls--;
            }
        }

        /**
         * Check whether or not all sessions have been closed. Ensure that the count to
         * close() is greater than or equal to the number of session creates.
         */
        public boolean haveAllSessionsBeenClosed() {
            return sessionCloseCalls >= sessionCreateCalls;
        }
    }
}

4 Replies to “The Value in Checking Your Style”

  1. An entirely valid point Gerrod if you knew the value for that Boolean at compile time. I had actually intended for the code above to be something more like:

    Boolean isReundant = Boolean.valueOf(someOtherRuntimeCondition);

    Thanks for the pickup.

  2. Pingback: Oliver's Blog

Comments are closed.