Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

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 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 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.

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.

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.
Post Undeleted by Tunaki
added 2050 characters in body
Source Link
Tunaki
  • 9.3k
  • 1
  • 31
  • 46

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 insteinstead 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.

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 inste

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.
Post Deleted by Tunaki
Source Link
Tunaki
  • 9.3k
  • 1
  • 31
  • 46

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 inste

lang-java

AltStyle によって変換されたページ (->オリジナル) /