CSV file download in Spring

Question

I recently worked on a module, in which I implemented the functionality of downloading the CSV files.

@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);
 }
}
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();
 }
}

I am using OpenCSV library for reading and writing the CSV files, in addition, one thing should be note down here is, the function writeNext() accepts String[] .

I initialized the String[] inside writeNext(). I want to know the better way to do the same, so the readability enhanced.

Please review my code and suggest me the best practice.

Answer 1

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 !

• user10753505: Really appreciate MadJlzz, I have one question regardign this, I am looking a way to make it universal, so i can pass only the list of object, and it will parse the data and write to csv
• MadJlzz: It's gonna be hard to do this universal without getting uneasy to read because you have to know what you want to write before actually writting. The headers won't be the same for example plus you'd have something different for UserInfo attributes. Seems complicated. Plus this is a feature for your frontend so you can't add too much technicals informations in your endpoints to help you with that.