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.)
3 Answers 3
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');
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. APreparedStatement
is created by callingprepareStatement(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 atry-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 users.getDouble(...)
. This would simplify the code. Also, you are parsingreceivedtime
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.
-
\$\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. Thereceivetime
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 theif
statement so I can use it in the else. I made two try-with-resourcestry (PreparedStatement stmt = .. )
andtry (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– Charly Roch2016年02月02日 10:13:23 +00:00Commented Feb 2, 2016 at 10:13
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.