So You Want to Practice your Code Reviewing Skills?

The code below is taken from a real web-app that has been up and running for > 7 years. This specific fragment is realizing the visitor count functionality: keeping track on the number of visitors hitting the site. Each time a new session is created SiteInfo.instance().addSession() is called.

Your task (should you choose to accept it...) is to find bugs in this code. In other words: will the visitor count, (as reported by SiteInfo.visitorCount()) always be correct? If not, what values can be seen there? How can we fix the code?

Please ignore design issues (I don't like the singleton anymore than you do), or technological issues, such as: "well you should just rewrite the whole thing with Spring + Hibernate". Just examine the code-as is and try to determine if/where can it fail.

public class SiteInfo {
   
   private static SiteInfo inst;
      
   private SiteInfo() {
      //
   };
   
   // This is a singleton (Yak!)
   public static SiteInfo instance() {
      if(inst == null) 
         inst = new SiteInfo();
      return inst;
   }

   private int sessionCounter = 0; 
   private static final String INIT = "100";
      
   public int visitorCount() {
      return sessionCounter;
   }
      
   public synchronized void reloadParamsFromDB() {
      Connection con = null;
      ConnectionPool pool = null;
      try {

         pool = ConnectionPoolFactory.getInstance();
         con = pool.getConnection();

         String countStr = DataBaseHelper.get(con, "config", "value",
            "param", "session_count"); 
         if (countStr == null) {
            String q = "INSERT INTO config VALUES ('session_count','" + INIT + "')";
            try {
               runCommand(q, con);
               countStr = INIT;
            }
            catch (Exception e) {
               log.error("", e);
            }
         }

         sessionCounter = Integer.parseInt(countStr);
      }
      catch (Exception e) {
         log.error("", e);
      }
      finally {
         ConnectionPoolFactory.release(pool, con);
      }
   }
   
   private void runCommand(String q, Connection con) throws Exception {
      Statement stmt = con.createStatement();
      try {
         stmt.executeUpdate(q);         
      }
      finally {
         stmt.close();
     }
   }
   
   public synchronized void addSession() {
      sessionCounter++;
      if(sessionCounter % 100 != 0)
         return;

      Connection con = ConnectionPoolFactory.getInstance().getConnection();
      Statement stmt = null;
      try {
         stmt = con.createStatement();
         String sql = "UPDATE config SET value='" + sessionCounter 
            + "' WHERE param='SESSION_COUNT'";
         stmt.execute(sql);
      }
      catch (Exception ex) {
         log.error("", ex);
      }
      finally {
         try {
            if(stmt != null)
               stmt.close();
            ConnectionPoolFactory.release(con);
         }
         catch(SQLException e) {
            log.error("", e);
         }
      }
   }   
}

public class DataBaseHelper {
   public static String get(Connection con, String table, 
      String columnA, String columnB, String columnAValue) {
      String result = "";
      String sqlQuery = "SELECT " + columnB + " from " + table
            + " WHERE " + columnA + " = '" + columnAValue + "'";

      // Run the query. Translate the result set into a list of maps.
      // Each map corresponds to a single row in the ResultSet
      List<Map<Object,Object>> rows = generateResult(sqlQuery, con);
      try {
         Iterator<Map<Object,Object>> iter = rows.iterator();
         if(iter.hasNext()) {
           Map<Object,Object> m = iter.next();
         
           result = (String) (m.get(columnB));
           if (result == null)
              return null;
         }
      } 
      catch (Exception e) {
         return null;
      }
      return result;
   }  

   public static List<Map<Object,Object>> generateResult(String query, Connection con) {
      List<Map<Object,Object>> result = new ArrayList<Map<Object,Object>>();
      try {
         ResultSet resultSet = DataBase.performSqlQuery(query, con);
         if(resultSet == null)
            throw new Exception("Impossible"); 
            
         ResultSetMetaData resultSetMetaData = resultSet.getMetaData();         
         int columnCount = resultSetMetaData.getColumnCount();
         
         String[] columnNames = new String[columnCount];
         for(int i = 0; i < columnCount; i++)
            columnNames[i] = resultSetMetaData.getColumnName(i + 1);
            
         while(resultSet.next()) {
            Map<Object,Object> map = new HashMap<Object,Object>();
            for(int i = 0; i < columnCount; i++) {
               String col = columnNames[i];
               map.put(col, resultSet.getString(i + 1));
            }

            result.add(map);
         }
      }
      catch(Exception e) {
         sLogger.error("", e);
      }

      return result;
   }
}