4
\$\begingroup\$

I have a database containing a high amount of data. I want my app to filter and select only some of it, and save it into a CSV file or JSON.

For the moment I have this method which takes the data from the database and puts it in a CSV:

void viewAllCounters (String tableName, long startDate, long endDate, Connection c) throws SQLException {
    Statement stmt = null;
    String query = "SELECT receivedtime, source, numulethbytesinter, numdlethbytesinter, numulethbytessent, numdlethbytessent, "
            + "numuniquegtpteids, numulbytessenttoaccl, numdlbytessenttoaccl "
            + " FROM " + tableName + " WHERE CAST(receivedtime AS integer) BETWEEN "+ startDate +" AND " + endDate +" ORDER BY receivedtime;";

    try {
        stmt = c.createStatement();
        ResultSet rs = stmt.executeQuery(query);
        int i = 0;
        double last = 0;
        DecimalFormat decFor = new DecimalFormat("0.000");
        SimpleDateFormat formatter = new SimpleDateFormat(DATE_FORMAT);

        try {
            File file = new File("KPI-"+tableName+"-"+ endDate +".csv");
            FileWriter fileWriter = new FileWriter(file);
            fileWriter.append("Date");
            fileWriter.append(',');
            fileWriter.append("Source");
            fileWriter.append(',');
            fileWriter.append("AUIDR");
            fileWriter.append(',');
            fileWriter.append("ADIDR");
            fileWriter.append(',');
            fileWriter.append("AUEDR");
            fileWriter.append(',');
            fileWriter.append("ADEDR");
            fileWriter.append(',');
            fileWriter.append("AUTPG");
            fileWriter.append(',');
            fileWriter.append("ADTPG");
            fileWriter.append(',');
            fileWriter.append("AST");
            fileWriter.append('\n');


            while (rs.next()) {
                if (last != 0){
                    String receivedtime = rs.getString("receivedtime");
                    String source = rs.getString("source");

                    double numulethbytesinter = Double.parseDouble(rs.getString("numulethbytesinter"));
                    double numdlethbytesinter = Double.parseDouble(rs.getString("numdlethbytesinter"));
                    double numulethbytessent = Double.parseDouble(rs.getString("numulethbytessent"));
                    double numdlethbytessent = Double.parseDouble(rs.getString("numdlethbytessent"));

                    double numuniquegtptieds = Double.parseDouble(rs.getString("numuniquegtpteids"));
                    double numulbytessenttoaccl = Double.parseDouble(rs.getString("numulbytessenttoaccl"));
                    double numdlbytessenttoaccl = Double.parseDouble(rs.getString("numdlbytessenttoaccl"));

                    //period between this entry and the last one (in second)
                    if (Double.parseDouble(receivedtime) > last) {
                        double period = Double.parseDouble(receivedtime) - last;                 

                        double AUIDR = (8*numulethbytesinter)/(1000000*period);
                        double ADIDR = (8*numdlethbytesinter)/(1000000*period);
                        double AUEDR = (8*numulethbytessent)/(1000000*period);
                        double ADEDR = (8*numdlethbytessent)/(1000000*period);
                        double AUTPG, ADTPG;

                        if (numuniquegtptieds != 0){
                            AUTPG = (8*numulbytessenttoaccl)/(numuniquegtptieds*1000000*period);
                            ADTPG = (8*numdlbytessenttoaccl)/(numuniquegtptieds*1000000*period);
                        }else{
                            AUTPG = 0;
                            ADTPG = 0;
                        }

                        double AST = (8*(numdlbytessenttoaccl+numulbytessenttoaccl))/(1000000*period);

                        String dateReceived = formatter.format(new Date (Long.parseLong(receivedtime)*1000));

                        fileWriter.append(dateReceived);
                        fileWriter.append(',');
                        fileWriter.append(source);
                        fileWriter.append(',');
                        fileWriter.append("" + AUIDR);
                        fileWriter.append(',');
                        fileWriter.append("" + ADIDR);
                        fileWriter.append(',');
                        fileWriter.append("" +AUEDR);
                        fileWriter.append(',');
                        fileWriter.append("" + ADEDR);
                        fileWriter.append(',');
                        fileWriter.append("" + AUTPG);
                        fileWriter.append(',');
                        fileWriter.append("" +ADTPG);
                        fileWriter.append(',');
                        fileWriter.append("" + AST);
                        fileWriter.append('\n');

                    i++;
                    }
                } else {
                    last = Double.parseDouble(rs.getString("receivedtime"));
                }
            }
        fileWriter.flush();
    fileWriter.close();
        } catch (IOException e) {
            System.err.println(e.getClass().getName()+": "+e.getMessage() + "\n\n\n");
        }
    } catch (SQLException e ) {
         System.err.println(e.getClass().getName()+": "+e.getMessage() + "\n\n\n");    } 
    finally {
        if (stmt != null) { stmt.close(); }
    }
}

This method is in a class called Dbfunctions which is called in the main method of my main class. I could add more fields to get in the future.

As you can see I create a CSV file and add all the data. But I am wondering, is there any way to make it faster or at least more readable?

I thought to add all this data in a List and return this list to create this. I guess the best for me is a Queue? Fast to insert fast to remove and first in first out. I could create a Queue<Queue<String>> so I will have:

Queue(QueueIterration1{[String DateIterration1],[String SourceIterration1]...}, QueueIterration2{...}, ...).

So I will pop the QueueIterration1 then a for loop to pop each element. Then pop QueueIterration2 etc...

So my question is: is it a good idea? Speaking about performances, time to write all the data. And do you have any suggestions on how to improve my code?

(Now I have 4 methods like this one to take different data from the database.)

\$\endgroup\$
3
\$\begingroup\$

On the append

The append method returns the writer, so one improvement could be to leverage this and chain the calls together.

So, where you're currently doing this:

fileWriter.append("Date");
fileWriter.append(',');
fileWriter.append("Source");
fileWriter.append(',');
fileWriter.append("AUIDR");
fileWriter.append(',');
fileWriter.append("ADIDR");
fileWriter.append(',');
fileWriter.append("AUEDR");
fileWriter.append(',');
fileWriter.append("ADEDR");
fileWriter.append(',');
fileWriter.append("AUTPG");
fileWriter.append(',');
fileWriter.append("ADTPG");
fileWriter.append(',');
fileWriter.append("AST");
fileWriter.append('\n');

You can instead write it as:

fileWriter
  .append("Date")
  .append(',')
  .append("Source")
  .append(',')
  .append("AUIDR")
  .append(',')
  .append("ADIDR")
  .append(',')
  .append("AUEDR")
  .append(',')
  .append("ADEDR")
  .append(',')
  .append("AUTPG")
  .append(',')
  .append("ADTPG")
  .append(',')
  .append("AST")
.append('\n');
\$\endgroup\$
2
\$\begingroup\$

Here are the concerns I have about your current code:

  • You are building your SQL query by appending directly Strings

    String query = "SELECT receivedtime, source, numulethbytesinter, numdlethbytesinter, numulethbytessent, numdlethbytessent, "
        + "numuniquegtpteids, numulbytessenttoaccl, numdlbytessenttoaccl "
        + " FROM " + tableName + " WHERE CAST(receivedtime AS integer) BETWEEN "+ startDate +" AND " + endDate +" ORDER BY receivedtime;";
    

    This is not a good idea as it can lead to SQL injection. What you want to use instead is a PreparedStatement, that tackles with this problem. A PreparedStatement is created by calling prepareStatement(sql) on the connection. All the parameters are replaced with the ? placeholder.

    String query = "SELECT receivedtime, source, numulethbytesinter, numdlethbytesinter, numulethbytessent, numdlethbytessent, "
        + "numuniquegtpteids, numulbytessenttoaccl, numdlbytessenttoaccl "
        + " FROM " + tableName + " WHERE CAST(receivedtime AS integer) BETWEEN ? AND ? ORDER BY receivedtime;";
    
    PreparedStatement stmt = c.prepareStatement(query);
    stmt.setLong(1, startDate);
    stmt.setLong(2, endDate);
    ResultSet rs = stmt.executeQuery();
    

    You can't use a placeholder for the FROM part, unfortunately.

  • You could get rid of the finally clause by using a try-with-resources.
  • Consider making your SQL query a constant. What I mean by that is that you could make the SQL query a constant by declaring it private static final String.
  • You are using the pattern Double.parseDouble(rs.getString(...)) when you could simply use rs.getDouble(...). This would simplify the code. Also, you are parsing receivedtime multiple times in your code (I counted 4). Consider parsing it only once and reusing it.

There is one suggestion that I could make:

  • Consider writing more specialized methods. You could refactor the code writing to a file into a specific method. Or, you could even make a generic class CsvWriter that would handle writing the CSV file when you are adding elements to it.
\$\endgroup\$
  • \$\begingroup\$ Thanks a lot, I change the query to a prepare statement, but I will use it only in this method so I won't make it constant I didn't thought about rs.getDouble(...) of course I changed it. The receivetime is used three time as double and once as long that's why I had let it as string. So I changed to a double and declare it before the if statement so I can use it in the else. I made two try-with-resourcestry (PreparedStatement stmt = .. ) and try (FileWriter fileWriter = ...). For the CsvWriter method I agree, see the end of my question. I don't really know what should be the best. \$\endgroup\$ – Charly Roch Feb 2 '16 at 10:13
1
\$\begingroup\$

Not closing fileWriter if there is an error

File file = new File("KPI-"+tableName+"-"+ endDate +".csv");
FileWriter fileWriter = new FileWriter(file);
...
fileWriter.flush();
fileWriter.close();

You should always close your resources in a finally block or with a try-with-resources. This prevent bugging code when you don't have any file channels left.

File file = new File("KPI-"+tableName+"-"+ endDate +".csv");
try(FileWriter fileWriter = new FileWriter(file)){
    ...
    fileWriter.flush();
}

Confusing variable names

                double numulethbytesinter = Double.parseDouble(rs.getString("numulethbytesinter"));
                double numdlethbytesinter = Double.parseDouble(rs.getString("numdlethbytesinter"));
                double numulethbytessent = Double.parseDouble(rs.getString("numulethbytessent"));
                double numdlethbytessent = Double.parseDouble(rs.getString("numdlethbytessent"));
                double numuniquegtptieds = Double.parseDouble(rs.getString("numuniquegtpteids"));
                double numulbytessenttoaccl = Double.parseDouble(rs.getString("numulbytessenttoaccl"));
                double numdlbytessenttoaccl = Double.parseDouble(rs.getString("numdlbytessenttoaccl"));

All these variables have confusing simulair names, this makes it hard to follow them.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.