Multithreaded report generation

Question

I have a requirement where I have to generate some reports read from a database and write the data to Excel.

I want your help in reviewing the following things in my code:

1. Database connection
2. Multithreaded programming
3. Is my data structure Map<Integer, TreeMap<String,String>> proper?
4. What design pattern can I apply?

DB connection

import java.sql.CallableStatement;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ResourceBundle;
import java.util.Vector;

import javax.sql.DataSource;

public class RConnection {
    private Connection mConnection;
    private Statement mStatement;
    private static Object lockObject = new Object();
    private static int openCount = 0;
    private static int closeCount = 0;
    private static String dataSourceName = "jdbc/dtcuae";
    private static DataSource dataSource = null;
    private Vector statementVector= new Vector();
    private Vector resultSetVector= new Vector();


    public CallableStatement createCallableStatement(String sql) throws SQLException
    {
        CallableStatement cstmt = null;
        try
        {
            cstmt = this.getDBConnection().prepareCall(sql);
            statementVector.add(cstmt);
        }
        catch(SQLException e)
        {       
            e.printStackTrace();
            throw new SQLException("Can not create CallableStatement for sql "+e.getMessage());
        }
        return cstmt;
    }


    public ResultSet getResultSet(CallableStatement cstmt, int paramNo) throws SQLException
    {
        ResultSet rs=null;
        try
        {
            rs=(ResultSet) cstmt.getObject(paramNo);
            resultSetVector.add(rs);
        }
        catch(SQLException e)
        {
            e.printStackTrace();
            throw new SQLException("Can not retrieve ResultSet for this CallableStatement "+e.getMessage());
        }
        return rs;
    }


    public static int getOpenConnections()
    {
        return openCount;
    }
    public static int getCloseConnections()
    {
        return closeCount;
    }
    /**
    /**
     * Constructor being made Private, Singleton implementation
     */
    public RConnection()
    {
        this.connect();
    }








    /**
     * This is the function that is used to connect to the
     * database using jdbc
     */
    public void connect()
    {
        try
        {
            String errorString = "Error obtaining database connection.";
            final ResourceBundle BUNDLE = ResourceBundle.getBundle(AppConstants.CONFIG_PATH);

                try {
                    /*
                    System.out.println("-------------------Oracle Connection Properties Start-----------------------------------");
                    System.out.println("oracle.jdbc.url"));
                    System.out.println("oracle.jdbc.usr"));
                    System.out.println("-------------------Oracle Connection Properties End-----------------------------------");
                    */
                    String DB_USERNAME = BUNDLE.getString(AppConstants.DB_USERNAME);
                    //String DB_PASSWORD = BUNDLE.getString(AppConstants.TR_DB_PASSWORD);

                    String DB_PASSWORD = AES.decryptString(BUNDLE.getString(AppConstants.DB_PASSWORD),
                            AppConstants.ENCRYPTION_KEY); 
                    String DB_URL = BUNDLE.getString(AppConstants.DB_URL);
                    Class.forName(AppConstants.DRIVER_CLASS_NAME);
                    mConnection = DriverManager.getConnection(DB_URL, DB_USERNAME,
                            DB_PASSWORD);
                    mConnection.setAutoCommit(false);       
                }catch (NullPointerException e) {                   
                    throw new Exception(errorString+":"+e.getMessage());
                }
                catch(SQLException sqle) {
                    throw new SQLException(errorString+":"+sqle.toString());
                }


            if (mConnection == null) {
                throw new SQLException(errorString);
            } else {
                synchronized (lockObject) {
                    openCount++;                    
                }
            }
        }
        catch(Exception ex)
        {
                ex.printStackTrace();
        }
    }

    /** Return Connection **/
    public Connection getDBConnection()
    {
        return mConnection;
    }

    /**
     * Execute any plain SQL query and returns the ResultSet
     * @throws SQLException 
     */
    public ResultSet executeSQL(String query) throws SQLException
    {
        ResultSet lRSet = null;
        try
        {
            // close the statement if already open
            if(mStatement != null) mStatement.close();
            mStatement = mConnection.createStatement();
            lRSet = mStatement.executeQuery(query);
        }
        catch(SQLException ex)
        {
            System.err.println("Error Query : " + query);
            ex.printStackTrace();
            throw new SQLException(ex.getMessage());
        }

        return lRSet;
    }

    public void commit()
    {
        try
        {
            if(mStatement != null) mStatement.close();
            if(mConnection != null && !mConnection.getAutoCommit()) mConnection.commit();
        }
        catch(Exception ex)
        {
            ex.printStackTrace();
        }
    }

    public void close()
    {
        try
        {
            if ((mConnection != null) && !mConnection.isClosed()) {
                if(!resultSetVector.isEmpty())
                {
                    //ok close all result sets once more
                    int rsSize = resultSetVector.size();
                    for(int i=0;i<rsSize;i++)
                    {
                        ResultSet rset=(ResultSet)resultSetVector.get(i);
                        try
                        {
                            rset.close();
                        }
                        catch(SQLException e)
                        {
                            throw new SQLException(e.getMessage());
                        }
                    }
                }

                if(!statementVector.isEmpty())
                {
                    //ok close all statements once more
                    int stmtSize = statementVector.size();
                    for(int i=0;i<stmtSize;i++)
                    {
                        CallableStatement cstmt=(CallableStatement)statementVector.get(i);
                        try
                        {
                            cstmt.close();
                        }
                        catch(SQLException e)
                        {
                            throw new SQLException(e.getMessage());
                        }
                    }
                }

                mConnection.close();
                synchronized (lockObject) {
                    closeCount++;                   
                }
            }
        }
        catch(Exception ex)
        {
            ex.printStackTrace();
        }
    }


    public void callDummyArgumentProcedure(String procedureName) {
        CallableStatement cstmt =null;
        try {
            cstmt = getDBConnection().prepareCall("{call " + procedureName + " (?)}");
            cstmt.setInt(1,0);
            cstmt.execute();
        }
        catch(Throwable e) {

        }
        finally {
            if(cstmt !=null) try {cstmt.close();}catch (SQLException e) {}
        }
    }


} 

Thread-based report generation

import java.util.Map;
import java.util.TreeMap;

public class ReportGenerator extends Thread {

    String countryName;

    public ReportGenerator(String countryName) {
        super();
        this.countryName = countryName;
    }


    public String getCountryName() {
        return countryName;
    }

    public void setCountryName(String countryName) {
        this.countryName = countryName;
    }

    public void run() {
        try {
            ReportDao dao = new ReportDao();
            System.out.println("Current Thread"
                    + Thread.currentThread().getName());
            Map<Integer, TreeMap<String, String>> data = dao.read(countryName);
            ExcelModifier.readExcel(data, countryName);

        } catch (Exception e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
    }
}

DAO

import java.lang.reflect.Field;
import java.sql.CallableStatement;
import java.sql.ResultSet;
import java.sql.ResultSetMetaData;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;

public class ReportDao {
    public RConnection connection = new RConnection();



    public static int getOracleParamReturnType(String paramName) {
        if (paramName == null)
            return -1;
        Field cursorField;
        try {
            Class c = Class.forName("oracle.jdbc.driver.OracleTypes");
            cursorField = c.getField(paramName);
            return cursorField.getInt(c);
        } catch (Throwable th) {
            th.printStackTrace();
            return -1;
        }
    }

    public Map<Integer, TreeMap<String,String>> read(String countryName) throws SQLException {
        System.out.println(" read");
        System.out.println("Country name data to be fetched:"+countryName);
        ResultSet rs = null;
        CallableStatement cstmt = null;
        try {
            String procedure = "{call USP_GET_KE_REPORT (?,?)}";
            connection.getDBConnection();
            cstmt = connection.createCallableStatement(procedure);
            cstmt.setString(1,countryName);
            cstmt.registerOutParameter(2, getOracleParamReturnType("CURSOR"));
            cstmt.execute();
            rs = connection.getResultSet(cstmt, 2);
            return populateData(rs);

        } finally {
            if (rs != null)
                rs.close();
            connection.close();
            System.out.println(" end ");
        }
    }

    public Map<Integer, TreeMap<String,String>> readPH(String countryName,String product) throws SQLException {
        System.out.println(" read");
        System.out.println("Country name data to be fetched:"+countryName);
        ResultSet rs = null;
        CallableStatement cstmt = null;
        try {
            String procedure = "{call GETPHCERTIFICATIONREPORT (?,?,?)}";
            cstmt = connection.createCallableStatement(procedure);
            cstmt.setString(1,product);
            cstmt.setString(2,countryName);
            cstmt.registerOutParameter(3, getOracleParamReturnType("CURSOR"));
            cstmt.execute();
            rs = connection.getResultSet(cstmt, 3);
            return populateData(rs);
        } finally {
            if (rs != null)
                rs.close();
            connection.close();
            System.out.println(" end ");
        }
    }

    private Map<Integer, TreeMap<String,String>> populateData(ResultSet rs) throws SQLException{
        Map<Integer, TreeMap<String,String>>  data = new TreeMap<Integer, TreeMap<String,String>>();

        List<String> coulmnNames = new ArrayList<String>();
        ResultSetMetaData rsmd = rs.getMetaData();
        int columnCount = rsmd.getColumnCount();
        for (int i = 1; i < columnCount + 1; i++ ) {
            coulmnNames.add(rsmd.getColumnName(i)) ;
        }
        int noOfRecords =0 ;
        while (rs.next()) {
            TreeMap<String,String> values = new TreeMap<String, String>();
            for (String name : coulmnNames) {
                    values.put(name, rs.getString(name));   
            }
            data.put(++noOfRecords, values);
        }
        System.out.println("---------No of records--"+noOfRecords);
        return data;
    }

}

Write data to Excel

import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.text.DateFormat;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.Iterator;
import java.util.Map;
import java.util.TreeMap;

import org.apache.poi.hssf.usermodel.HSSFCellStyle;
import org.apache.poi.hssf.usermodel.HSSFSheet;
import org.apache.poi.hssf.usermodel.HSSFWorkbook;
import org.apache.poi.hssf.util.HSSFColor;
import org.apache.poi.ss.usermodel.Cell;
import org.apache.poi.ss.usermodel.RichTextString;
import org.apache.poi.ss.usermodel.Row;

public class ExcelModifier {

    public static void readExcel(Map<Integer, TreeMap<String, String>> data,
            String countryName) throws IOException {
        String fileName = "c:\\AIG_KX_Certification Report_KE_Afghanistan_2003a5.xls";
        String destFileName = "C:\\AIG_KX_Certification Report_KE_"
                + countryName + "_2003a5.xls";
        FileInputStream file2 = new FileInputStream(new File(fileName));

        // Get the workbook instance for XLS file
        HSSFWorkbook workbook = new HSSFWorkbook(file2);

        // Get first sheet from the workbook
        HSSFSheet sheet = (HSSFSheet) workbook.getSheetAt(1);
        System.out.println("Sheet name before:" + workbook.getSheetName(1));
        workbook.setSheetName(1, countryName);
        System.out.println("Sheet name after:" + workbook.getSheetName(1));

        // if there is an validation error then write the error messages to
        // excel and quit

        // Get iterator to all the rows in current sheet
        HSSFCellStyle unlockedCellStyle = unLockCellStyle(workbook);

        HSSFCellStyle cs = greyCellStyle(workbook);
        int rowNumber = 0;
        int recordCount = data.size();
        System.out.println(" recordcount " + recordCount);
        System.out.println(" writin data to excel "
                + Thread.currentThread().getName());
        int currentRecord = 0;
        Iterator<Row> rowIterator = sheet.iterator();
        while (rowIterator.hasNext()) {
            Row row = (Row) rowIterator.next();
            Cell keCell = row.getCell(2);
            RichTextString excelValue = null;
            if (keCell != null) {
                excelValue = keCell.getRichStringCellValue();
            }
            ++rowNumber;
            if (rowNumber <= 8) {
                continue;
            }
            ++currentRecord;
            Map<String, String> map = data.get(currentRecord);
            boolean dummyData = false;
            if (map == null) {
                dummyData = true;
                System.err.println(" NO RECORD " + countryName + " rowNumber "
                        + currentRecord);
                continue;
            }

            keData(dummyData, map, row, unlockedCellStyle, cs);

        }

        if (recordCount > (rowNumber - 8)) {
            createRow(rowNumber++, currentRecord++, sheet, data,
                    unlockedCellStyle, cs);
        }

        sheet.setColumnHidden(0, true);
        sheet.setColumnHidden(1, true);
        sheet.setColumnHidden(4, true);
        sheet.setColumnHidden(6, true);
        sheet.setColumnHidden(8, true);
        sheet.setColumnHidden(10, true);
        sheet.setColumnHidden(12, true);
        sheet.setColumnHidden(14, true);
        sheet.setColumnHidden(18, true);

        FileOutputStream fileOut = new FileOutputStream(destFileName);
        workbook.write(fileOut);
        fileOut.flush();
        fileOut.close();

    }

    public static void createPHExcel(
            Map<Integer, TreeMap<String, String>> data, String countryName,
            String product) throws IOException {
        String fileName = "c:\\AIG_KX_Certification Report-PH v2.4.3 - 08-27-2014a.xls";
        String destFileName = "C:\\AIG_KX_Certification Report-PH-GL-"
                + countryName + ".xls";

        FileInputStream file2 = new FileInputStream(new File(fileName));

        // Get the workbook instance for XLS file
        HSSFWorkbook workbook = new HSSFWorkbook(file2);

        // Get first sheet from the workbook
        HSSFSheet sheet = (HSSFSheet) workbook.getSheetAt(1);
        System.out.println("Sheet name before:" + workbook.getSheetName(1));
        workbook.setSheetName(1, countryName);
        System.out.println("Sheet name after:" + workbook.getSheetName(1));
        Map<String, String> headermap = data.get(1);
        System.out.println(" header map " + headermap);

        Row countryRow = sheet.getRow(0);
        Cell countryCell = countryRow.getCell(2);
        countryCell.setCellValue(countryName);

        Row productRow = sheet.getRow(3);
        Cell productCell = productRow.getCell(2);
        productCell.setCellValue(product);

        Row providertRow = sheet.getRow(4);
        Cell providerCell = providertRow.getCell(2);
        providerCell.setCellValue(headermap.get("PROVIDED_BY"));

        Row providertTitleRow = sheet.getRow(5);
        Cell providertTitleCell = providertTitleRow.getCell(2);
        providertTitleCell.setCellValue(headermap.get("ROVIDER_TITLE"));

        Row approverRow = sheet.getRow(6);
        Cell approverCell = approverRow.getCell(2);
        approverCell.setCellValue(headermap.get("APPROVED_BY"));

        Row approverTitleRow = sheet.getRow(7);
        Cell approverTitleCell = approverTitleRow.getCell(2);
        approverTitleCell.setCellValue(headermap.get("APPROVED_BY_TITLE"));

        Row effectiveStartDateRow = sheet.getRow(8);
        Cell effectiveStartDateCell = effectiveStartDateRow.getCell(2);

        effectiveStartDateCell.setCellValue(convertStringToDate(headermap
                .get("EFFECTIVE_START_DT")));

        // if there is an validation error then write the error messages to
        // excel and quit

        // Get iterator to all the rows in current sheet
        HSSFCellStyle unlockedCellStyle = unLockCellStyle(workbook);

        int rowNumber = 0;
        int recordCount = data.size();
        System.out.println(" recordcount " + recordCount);
        System.out.println(" writin data to excel "
                + Thread.currentThread().getName());
        int currentRecord = 0;
        Iterator<Row> rowIterator = sheet.iterator();
        while (rowIterator.hasNext()) {
            Row row = (Row) rowIterator.next();
            Cell keCell = row.getCell(2);
            // System.out.println(" KE Name "+excelValue);

            ++rowNumber;
            System.out.println("rowNumber " + rowNumber);
            if (rowNumber < 12) {
                continue;
            }
            ++currentRecord;
            Map<String, String> map = data.get(currentRecord);
            boolean setDummyValue = false;
            if (recordCount < rowNumber) {
                setDummyValue = true;
            }

            int cellNo = 1;
            Iterator<Cell> cells = row.cellIterator();
            while (cells.hasNext()) {
                Cell cell = (Cell) cells.next();
                switch (cellNo) {
                case 1:
                    setStringCellValue(setDummyValue ? "" : map
                            .get("PH_CATEGORY_NM"), cell, unlockedCellStyle);
                    break;
                case 2:
                    setStringCellValue(setDummyValue ? "" : map
                            .get("CATEGORY_DISPLAY_ORDER"), cell,
                            unlockedCellStyle);
                    break;
                case 3:
                    setStringCellValue(setDummyValue ? "" : map
                            .get("SUB_CATEGORY_NM"), cell, unlockedCellStyle);
                    break;
                case 4:
                    setStringCellValue(setDummyValue ? "" : map
                            .get("SUB_CATEGORY_DISPLAY_ORDER"), cell,
                            unlockedCellStyle);
                    break;
                case 5:
                    setStringCellValue(setDummyValue ? "" : map
                            .get("QUESTION_NM"), cell, unlockedCellStyle);
                    break;
                case 6:
                    setStringCellValue(setDummyValue ? "" : map
                            .get("QUESTION_PRINT_ORDER"), cell,
                            unlockedCellStyle);
                    break;
                case 7:
                    setStringCellValue(setDummyValue ? "" : map
                            .get("ANSWER_VALUE"), cell, unlockedCellStyle);
                    break;
                case 8:
                    setStringCellValue(setDummyValue ? "" : map
                            .get("COMMENT_VALUE"), cell, unlockedCellStyle);
                    break;
                case 9:
                    setStringCellValue(setDummyValue ? "" : map
                            .get("ANSWER_VALUE1"), cell, unlockedCellStyle);
                    break;
                case 10:
                    setStringCellValue(setDummyValue ? "" : map
                            .get("COMMENT_VALUE1"), cell, unlockedCellStyle);
                    break;
                default:
                    break;
                }
                cellNo++;
            }

        }

        if (recordCount > rowNumber) {
            System.err.println(" More Rows required");
        }
        /*
         * sheet.setColumnHidden(0, true); sheet.setColumnHidden(1, true);
         * sheet.setColumnHidden(4, true); sheet.setColumnHidden(6, true);
         * sheet.setColumnHidden(8, true); sheet.setColumnHidden(10, true);
         * sheet.setColumnHidden(12, true); sheet.setColumnHidden(14, true);
         * sheet.setColumnHidden(18, true);
         */

        FileOutputStream fileOut = new FileOutputStream(destFileName);
        workbook.write(fileOut);
        fileOut.flush();
        fileOut.close();

    }

    private static String replcaseHtmlTags(String value) {
        if (value != null) {
            value = value.replaceAll("<br>", "\n");
        } else {
            value = "";
        }
        return value;
    }

    public static String convertDateToString(Date date, String formatToConvert)
            throws SystemException {

        SimpleDateFormat dateFormat = null;
        String formatedStr = "";
        try {
            dateFormat = new SimpleDateFormat(formatToConvert);
            formatedStr = dateFormat.format(date);
        } catch (NullPointerException ne) {
            ne.printStackTrace();
        } catch (IllegalArgumentException ire) {
            ire.printStackTrace();
        }
        return formatedStr;
    }

    public static String convertStringToDate(String dateAsString) {
        String start_dt = dateAsString;
        DateFormat formatter = new SimpleDateFormat("yyyy-MM-DD");
        Date date = null;
        try {
            date = (Date) formatter.parse(start_dt);
        } catch (ParseException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        SimpleDateFormat newFormat = new SimpleDateFormat("MM-dd-yyyy");
        String finalString = newFormat.format(date);
        return finalString;
    }

    private static HSSFCellStyle unLockCellStyle(HSSFWorkbook workbook) {
        HSSFCellStyle unlockedCellStyle = workbook.createCellStyle();
        unlockedCellStyle.setWrapText(true);
        unlockedCellStyle.setLocked(false);
        unlockedCellStyle.setBorderLeft(HSSFCellStyle.BORDER_THIN);
        unlockedCellStyle.setBorderRight(HSSFCellStyle.BORDER_THIN);
        unlockedCellStyle.setBorderTop(HSSFCellStyle.BORDER_THIN);
        unlockedCellStyle.setBorderBottom(HSSFCellStyle.BORDER_THIN);
        return unlockedCellStyle;
    }

    private static HSSFCellStyle greyCellStyle(HSSFWorkbook workbook) {
        HSSFCellStyle cs = workbook.createCellStyle();
        cs.setFillForegroundColor(HSSFColor.GREY_40_PERCENT.index);
        cs.setFillPattern(HSSFCellStyle.SOLID_FOREGROUND);
        cs.setLocked(false);
        cs.setWrapText(true);
        cs.setBorderLeft(HSSFCellStyle.BORDER_THIN);
        cs.setBorderRight(HSSFCellStyle.BORDER_THIN);
        cs.setBorderTop(HSSFCellStyle.BORDER_THIN);
        cs.setBorderBottom(HSSFCellStyle.BORDER_THIN);
        return cs;
    }

    private static void setStringCellValue(String value, Cell cell,
            HSSFCellStyle unlockedCellStyle) {
        cell.setCellValue(replcaseHtmlTags(value));
        cell.setCellStyle(unlockedCellStyle);
    }

    private static void createRow(int rowNumber, int recordNumber,
            HSSFSheet sheet, Map<Integer, TreeMap<String, String>> data,
            HSSFCellStyle unlockedCellStyle, HSSFCellStyle cs) {
        int nextRow = 12;
        System.out.println(" creating row");
        Row r = sheet.getRow(rowNumber);
        if (r == null) {
            r = sheet.createRow(nextRow);
        }
        Map<String, String> map = data.get(recordNumber);
        keData(false, map, r, unlockedCellStyle, cs);

    }

    private static void keData(boolean dummyData, Map<String, String> map,
            Row row, HSSFCellStyle unlockedCellStyle, HSSFCellStyle cs) {
        int cellNo = 1;
        Iterator<Cell> cells = row.cellIterator();
        while (cells.hasNext()) {
            Cell cell = (Cell) cells.next();
            switch (cellNo) {
            case 1:
                cell.setCellValue(dummyData ? "" : map.get("ID"));
                cell.setCellStyle(unlockedCellStyle);
                break;
            case 2:
                cell.setCellValue(dummyData ? "" : map.get("SEQ_NO"));
                cell.setCellStyle(unlockedCellStyle);
                break;
            case 3:
                cell.setCellValue(dummyData ? "" : map.get("KE_NM"));
                cell.setCellStyle(cs);
                break;
            case 4:
                cell.setCellValue(dummyData ? "" : map.get("KE_DS"));
                cell.setCellStyle(cs);
                break;
            case 5:
                cell.setCellValue(dummyData ? "" : map.get("PRODUCT1"));
                cell.setCellStyle(unlockedCellStyle);
                break;
            case 6:
                cell.setCellValue(dummyData ? "" : map.get("PRODUCT2"));
                cell.setCellStyle(unlockedCellStyle);
                break;
            case 7:
                cell.setCellValue(dummyData ? "" : map.get("STATE1"));
                cell.setCellStyle(unlockedCellStyle);
                break;
            case 8:
                cell.setCellValue(dummyData ? "" : map.get("STATE2"));
                cell.setCellStyle(unlockedCellStyle);
                break;
            case 9:
                cell.setCellValue(dummyData ? "" : map.get("ENTITY_ORIGINAL"));
                cell.setCellStyle(unlockedCellStyle);
                break;
            case 10:
                cell.setCellValue(dummyData ? "" : map.get("ENTITY"));
                cell.setCellStyle(unlockedCellStyle);
                break;
            case 11:
                String shortAswere1 = map.get("SHORT ANSWER VALUE1");
                shortAswere1 = shortAswere1 == null ? "" : shortAswere1
                        .replaceAll("<br>", "\n");
                cell.setCellValue(dummyData ? "" : shortAswere1);
                cell.setCellStyle(unlockedCellStyle);
                break;
            case 12:
                String shortAswere2 = map.get("SHORT ANSWER VALUE2");
                shortAswere2 = shortAswere2 == null ? "" : shortAswere2
                        .replaceAll("<br>", "\n");
                cell.setCellValue(dummyData ? "" : shortAswere2);
                cell.setCellStyle(unlockedCellStyle);
                break;
            case 13:
                String longAswere1 = map.get("LONG ANSWER VALUE1");
                longAswere1 = longAswere1 == null ? "" : longAswere1
                        .replaceAll("<br>", "\n");
                cell.setCellValue(dummyData ? "" : longAswere1);
                cell.setCellStyle(unlockedCellStyle);
                break;
            case 14:
                String longAswere2 = map.get("LONG ANSWER VALUE2");
                longAswere2 = longAswere2 == null ? "" : longAswere2
                        .replaceAll("<br>", "\n");
                cell.setCellValue(dummyData ? "" : longAswere2);
                cell.setCellStyle(unlockedCellStyle);
                break;
            default:
                break;
            }
            cellNo++;
        }
    }
}

Answer 1

That's a lot of code :)

public RConnection()
{
    this.connect();
}

This is a bad. There are a number of examples, but basically you could subclass RConnection, implement connect to use a member variable of RConnection and it would still be null. see: https://stackoverflow.com/questions/18138397/calling-method-from-constructor if you make connect final it would be safe, but still not ideal.

in connect() :

It looks like you want connect to handle reference counting. However you check if mConnect already exists, after you assign it a new connection.

you probably want:

public void connect() {
    if (mConnect != null) {
         // connection made, just update ref counter
         openCount++;
         return;
    }

    // no connection, set one up, set openCount to 1, etc

your exception handling needs some work I suspect, though I might be missing the intended flow you want here. I'll point out what I think you're doing and why you shouldn't, but if i'm wrong, feel free to poke fun at me.

    // some statements 
} catch (NullPointerException e) {                   
    throw new Exception(errorString+":"+e.getMessage());
} catch(SQLException sqle) {
    throw new SQLException(errorString+":"+sqle.toString());
}

if (mConnection == null) {
    throw new SQLException(errorString);
} else {
    synchronized (lockObject) {
        openCount++;                    
    }
}

To me it looks like you are trying to avoid calling openCount++ if any of the exceptions are thrown, by catching them and then rethrowing. To achieve the same result, you can simply move synchronized (lockObject) { openCount++ } inside the try block. if an exception occurs, it will be skipped. (so under // some statements, in the above)

Additionally, catching an SQLException and then rethrowing it as another SQLException is wasteful as you lose your stacktrace information. If you really must prefix the message with "Error obtaining database connection." (which doesn't add much information I think) then at least also transfer the stacktrace. However I strongly recommend to simply rethrow the original exception or not catching it on that level.

Catching the nullpointer exception: pretty much the same story, with the addition that NullPointerExceptions are one of the few exceptions you can almost always avoid by changing your code. It looks to me like you're afraid of triggering a nullpointer and wanting to catch it. After rescanning your code, I don't see any place where you could encounter null. The driver manager returns a connection or throws an exception. A fair few people are worried about null values when they are unsure, but the uncertainty usually propagates upwards, every level testing for null.

Your rethrown exceptions by the way only print their stacktrace and then get absorbed. the end user can never tell something went wrong. Is this intentional? it might be better to declare a ConnectionException of your own, and throw that when you receive an sql exception (or simply throw the sql exception directly).

if (mConnection == null) {
    throw new SQLException(errorString);
}

I don't think this ever happens. Either an exception is thrown or mConnection is set.

public ResultSet executeSQL(String query) throws SQLException
{
    ResultSet lRSet = null;
    try
    {
        // close the statement if already open
        if(mStatement != null) mStatement.close();
        mStatement = mConnection.createStatement();
        lRSet = mStatement.executeQuery(query);
    }
    catch(SQLException ex)
    {
        System.err.println("Error Query : " + query);
        ex.printStackTrace();
        throw new SQLException(ex.getMessage());
    }

    return lRSet;
}

You are returning an internal resultset, which might not be closed by the calling method. Usually you want to use the resultset in the same scope it was created. At least that's the rule with files. it is hard to trace unclosed sets. In the first part, you keep vectors with statements/resultsets to close when the db connection is closed, but this isn't used here.

throw new SQLException(ex.getMessage()) loses information. (stacktrace) why not do:

catch (SQLException ex) {
    // do logging as you like
    // then when you're done, simply rethrow it
    throw ex;
}

Also, you are opening / closing mStatement. is there a reason for this? Why not create a new statement whenever this method gets called? I don't see what you gain by making it an instance variable when it can simply be a stack variable. The resultset that is attached to the statement will be closed when the statement is closed (http://docs.oracle.com/javase/6/docs/api/java/sql/Statement.html)

This ties in with not returning a resultset I guess, but returning the values inside. But this depends as you might have a large amount of rows /data that you don't want to get out.

However this also brings me to my next point, this class is not thread safe. Make sure that is listed in the class javadoc. if 2 threads call executeSQL (or call connect() or a bunch of other methods) then you will get some odd results. (resultsets already being closed for starters)

I'm pointing this out because you have synchronized blocks so there is a suggestion you expect to be called in a multithreaded environment.

Overall, I think you want a utility method that creates a connection, with some utility code that handles the standard work for performing queries. You can make this a lot easier if you don't try to keep all this as instance variables.

for instance you could do something like;

public interface ResultHandler {
    public void resultCallBack(ResultSet rs);
}

public static Connection getConnection() throws SQLException {
    // like connect(), but simply construct a new connection.
    // and return it. since you're using a datasource, the connection should come
    // from a pool (likely but doesn't _have_ to be)
}

public static void executeQuery(String query, ResultHandler handler) throws SQLException {
    Connection connection = getConnection();
    Statement statement = conn.createStatement();
    ResultSet result = statement.executeQUery(query);
    while (result.next()) {
        handler.resultCallBack(result);
    }
    result.close(); // also closed by statement, but for symmetry. 
    statement.close();
    connection.close();
    // yes i've been lazy and left out the catch exception / finally / if (result != null) etc. 
    // this is meant as pseudocode to highlight the resulthandler part.
}

// and then where you need it you can call:
public void someMethod() {
    final List<String> result = new ArrayList<String>();
    SQLUtil.executeQuery("some query", new ResultHandler() {
        public void resultCallBack(ResultSet rs) { 
            result.add(rs.getString(0));
        }
    });
}

Anyways this may be over the top but I think if you just want utility functions and the certainty your results and statements are closed, then you could consider doing something like the above, rather than have an instance variable that overwrites it's statement.

One more thing, you are keeping 1 single connection at the moment, but your datasource might represent a pool of connections for you to use. your current implementation only let's you use the 1 connection. If you plan to be multithreaded this might mean all threads try to use the 1 connection for data access. You have a racecondition in connect() (connection might be overwritten by other thread if the calls are close together) so that's not ideal.

so far, your dao's get instantiated on the stack (not static or otherwise accessible to multiple threads) and the connection is inside, so this shouldn't be a big issue.

moving on..

public Map<Integer, TreeMap<String,String>> read(String countryName) throws SQLException {
    System.out.println(" read");
    System.out.println("Country name data to be fetched:"+countryName);
    ResultSet rs = null;
    CallableStatement cstmt = null;
    try {
        String procedure = "{call USP_GET_KE_REPORT (?,?)}";
        connection.getDBConnection();
        cstmt = connection.createCallableStatement(procedure);
        cstmt.setString(1,countryName);
        cstmt.registerOutParameter(2, getOracleParamReturnType("CURSOR"));
        cstmt.execute();
        rs = connection.getResultSet(cstmt, 2);
        return populateData(rs);

    } finally {
        if (rs != null)
            rs.close();
        connection.close();
        System.out.println(" end ");
    }
}

System.out : I would really remove this and introduce a good logger that you can configure to ignore info statements and just print warning/error. that way you can distinguish between important runtime info (exceptions mostly) and debug output.

other than that:

connection.getDBConnection() looks like a getter that doesn't return anything. perhaps a rename to "connection.establishDBConnection" to take a step away from looking like a getter. CallableStatement cstmt = null; is not used outside the try { } block so could be moved inside there. the return argument i'll get back to later

moving on:

private Map<Integer, TreeMap<String,String>> populateData(ResultSet rs) throws SQLException{
    Map<Integer, TreeMap<String,String>>  data = new TreeMap<Integer, TreeMap<String,String>>();

    List<String> coulmnNames = new ArrayList<String>();
    ResultSetMetaData rsmd = rs.getMetaData();
    int columnCount = rsmd.getColumnCount();
    for (int i = 1; i < columnCount + 1; i++ ) {
        coulmnNames.add(rsmd.getColumnName(i)) ;
    }
    int noOfRecords =0 ;
    while (rs.next()) {
        TreeMap<String,String> values = new TreeMap<String, String>();
        for (String name : coulmnNames) {
                values.put(name, rs.getString(name));   
        }
        data.put(++noOfRecords, values);
    }
    System.out.println("---------No of records--"+noOfRecords);
    return data;
}

you use

Map< Integer, TreeMap<.... 

why not use Map> ? is it important to make users of your method aware of the implementation? do they need to call a tree function? I'd recommend using

Map<Integer, Map<String, String>>

the usual comment about replace sys.out with log.error applies here. Also I think the ++noOfRecords is implicit information. If you use List> then the iteration of the list == index of result. (well ok it'd be 0 based but you can make that 1-based easily)

It would remove 1 level of indirection and simplify. It looks like you'll always read out the entire resultset, so then you definitely want to use a utility method. If you always read them out in this manner, then you don't even need the resulthandler interface I suggested, you can leave that out and instead do

public static Connection getConnection() throws SQLException {
    // like connect(), but simply construct a new connection.
    // and return it. 
}

public static List<Map<String,String>> executeQuery(String query) throws SQLException {
    List<Map<String,String>> resultList = new ArrayList<Map<String,String>>();
    Connection con = getConnection();
    Statement statement = conn.createStatement();
    ResultSet result = statement.executeQUery(query);
    while (result.next()) {
        resultList.add(transformToMap(result));
    }
    result.close(); // also closed by statement, but for symmetry. 
    statement.close();
    connection.close();
}

public static Map<String,String> transformToMap(ResultSet resultSet) {
    // creat treemap, fill, etc. 
    // if you want to save querying the metadata (which isn't expensive, but if you want)
    // then you can make the columnlist an input argument.
}

anyways enough suggestions, don't mean to push my way of doing things onto you, but wanted to show that you can make these utility methods cover the result you want.

back to your method; No it's not bad to return a map if you know the results are all strings and you need a generic form to return them.

Hope it wasn't too much. As I pointed out before, I would restructure the code and strip out the vectors with results/statements, would make the getConnection static and return a new connection each call rather than reuse. That way the connection leaves on each threads stack and won't interfere with other threads trying to use it at the same time.

you have a good generic way to read the resultset (though limited to strings but that's not bad). I would suggest you write code so that you simply unpack the result into that form when you first get the resultset, so that then you can close the resultset in the same scope it was openened.

Answer 2

I am not doing a full review due to the length of your code, but here are a few random observations:

• You are using Vector, which I have not seen in Java since the 90's. You are not using generics, which I have not seen for a decade.

• ExcelModifier is just a series of static methods, which usually means you did not take enough time to think about your OO design.

• ExcelModifier.readExcel is not a good name. That method actually writes a file. Maybe you should rename it and have it return return some String which you can then save as a file.

• ExcelModifier.readExcel hard codes the input/output file names. You should never hard code anything. You should pass those as parameters to the method.

• Some methods of ExcelModifier have very repetitive code. You can usually shorten that by being a bit clever. For example, you have a long switch statement with:

          case 1:
              setStringCellValue(setDummyValue ? "" : map
                      .get("PH_CATEGORY_NM"), cell, unlockedCellStyle);
  

  but you could define a Map<Integer, String> cellTypeMap with 1 -> "PH_CATEGORY", etc. and call

   setStringCellValue(setDummyValue ? "" : map.get(cellTypeMap.get(cellNo), cell, unlockedCellStyle)
  

  I don't really like the business with setDummyValue and you can probably do something better on that front.

• map is not a sufficiently descriptive name.

• Verbs are for method names, so setDummyValue would be a method name, not a boolean. I'm sorry for nitpicking about variable names, but it is very important for code readability.

• ReportGenerator should be immutable, especially since this is multi-threaded. This just means that you make the countryName member final and remove the setter.

• You should not use a raw Thread as this is dated. Look into Executors instead.

• Your indentation style is not consistent: sometimes you use the Java convention, but sometimes the C++/C# convention.

As for your question about what design pattern you can use, you are basically just transforming data from a DB to a file. There is not much design here. But you could make your code clearer by completely separating (1) fetching the data from DB and (2) transforming the data and (3) writing the data to the filesystem.

You also wanted to know if your Map<Integer, TreeMap<String,String>> is appropriate. It's not. The integer just lists the number of the entry, so this is really a list: list.get(i) instead of map.get(i). It is clearer to use a list since you know it represents an ordered sequence of items.

Since you know the column names in advanced, you could define them in an enum. You can then replace TreeMap<String, String> by an EnumMap.