First of all, please don't take what I am writting personnaly. Also, I am sure there would be something to argue with my proposals so feel free to tell me!
Let's begin with:
@RequestMapping(value = "/export_data")
public void downloadDataInCsv(
@RequestParam("type") String type,
@RequestParam("tID") String tableId,
HttpServletRequest request,
HttpServletResponse response) throws IOException {
if (type.equals("csv")) {
List<UserInfo> list = userInfoDao.findById(tableId);
ExportCsvUtil.downloadCsv(request, response, list);
}
}
I hate using plain strings when doing some comparison especially when it looks like a real constant like csv
In order to remove the plain String, I usually use enum to do that.
Why not replace type.equals("csv") by something like type.equals(FileType.CSV.name())
Next, if we are looking here at this chunk:
List<UserInfo> list = userInfoDao.findById(tableId);
ExportCsvUtil.downloadCsv(request, response, list);
we see that your are proceding with ExportCsvUtil.downloadCsv() even if the list is empty. Should this be a normal behaviour of your application ? Maybe you should send a message telling the user that they were no data to retrieve ?
Last thing for downloadDataInCsv() method is that if your type isn't a csv file, it will return a HTTP code 200 with nothing. I think the client would be kind of confused with that. Maybe try to send a different message like I said before. (e.g: only CSV export is available at the moment)
Next chunk! :)
private void downloadCsv(HttpServletRequest request, HttpServletResponse response, List<UserInfo> list) throws IOException {
String headerKey = "Content-Disposition";
String headerValue = String.format("attachment; filename=Table_Data.csv");
response.setContentType("text/csv");
response.setHeader(headerKey, headerValue);
try (final CSVWriter writer = new CSVWriter(response.getWriter(), ",")) {
writer.writeNext(new String[]{"User Id", "First Name", "Last Name", "Roll No", "Email ID", "Gender"});
for (UserInfo entry: list) {
// cast/convert to String where needed
writer.writeNext(new String[]{entry.getUserId()+"", entry.getFirstName(), entry.getLastName(),entry.getRollNo(),entry.getEmail(),entry.getGender()});
}
writer.close();
}
}
If you look at String headerValue = String.format("attachment; filename=Table_Data.csv");, you don't use String.format() capabilities at all! (if I am not mistaken)
String.format() allows you to do things like String.format("Hello %s", name)
You can replace this with an old plain String then.
response.setContentType("text/csv");, if you are using the enum like I said before, you could add a property contentType that allows you to do FileType.CSV.getContentType() in order to remove the hardcoded string.
writer.writeNext(new String[]{"User Id", "First Name", "Last Name", "Roll No", "Email ID", "Gender"});, since you know the header of the file, why not store is as a constant so it is not recreated every time you call the endpoint ?
Like so:
private static final String[] CSV_HEADER = new String[]{"User Id", "First Name", "Last Name", "Roll No", "Email ID", "Gender"});
I think you could do something about the for loop with some of Java 8 fanciness but I am not confident enough to write this without tools for testing.
Lastly, you don't have to do writer.close() since you are opening your file with a try with resource
Basically, CSVWriter class should implement AutoCloseable wich call close() method for you at the end of the try statement so you do not have to worry about this.
Documentation about this is here
Hope my answer helps !
