Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use OpenCSV parsing library #48

Merged
merged 5 commits into from
Jun 16, 2020
Merged

Use OpenCSV parsing library #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4c2a953
close #47
tttzach Jun 16, 2020
b0d8b6d
Merge branch 'master' into OpenCSV
tttzach Jun 16, 2020
abdaa24
Use relative file path and remove redundant newline
tttzach Jun 16, 2020
ca01a53
Merge branch 'OpenCSV' of https://github.com/tttzach/step into OpenCSV
tttzach Jun 16, 2020
5f1f235
Use relative file path and remove redundant newline
tttzach Jun 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions portfolio/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@
<artifactId>appengine-api-1.0-sdk</artifactId>
<version>1.9.59</version>
</dependency>
<dependency>
<groupId>com.opencsv</groupId>
<artifactId>opencsv</artifactId>
<version>4.5</version>
</dependency>
</dependencies>

<build>
Expand Down
37 changes: 20 additions & 17 deletions portfolio/src/main/java/com/google/sps/servlets/CoronavirusDataServlet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@

package com.google.sps.servlets;

import com.opencsv.CSVReaderHeaderAware;
import com.google.gson.Gson;
import java.io.FileReader;
import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
import java.util.Scanner;
import javax.servlet.annotation.WebServlet;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
Expand All @@ -32,21 +33,8 @@ public class CoronavirusDataServlet extends HttpServlet {

@Override
public void init() {
Scanner scanner = new Scanner(getServletContext().getResourceAsStream(
"/WEB-INF/coronavirus-stats-by-country.csv"));
// Ignore header row = "Country/Region,Confirmed,Active,..."
if (scanner.hasNextLine()) {
scanner.nextLine();
}
while (scanner.hasNextLine()) {
// line = "Afghanistan,20917,369.0,..."
String line = scanner.nextLine();
String[] cells = line.split(",");
String country = String.valueOf(cells[0]);
Integer confirmedCases = Integer.valueOf(cells[1]);
coronavirusCases.put(country, confirmedCases);
}
scanner.close();
String filePath = "../../src/main/webapp/WEB-INF/coronavirus-stats-by-country.csv";
readCsv(filePath);
}

@Override
Expand All @@ -56,5 +44,20 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) thro
String json = gson.toJson(coronavirusCases);
response.getWriter().println(json);
}


private void readCsv(String filePath) {
try {
CSVReaderHeaderAware csvReader = new CSVReaderHeaderAware(new FileReader(filePath));
Map<String, String> line;
while ((line = csvReader.readMap()) != null) {
String country = line.get("Country/Region");
Integer confirmedCases = Integer.valueOf(line.get("Confirmed"));
coronavirusCases.put(country, confirmedCases);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Food for thought: if the code which processes the file throws an exception, then you will never call csvReader.close(). This can cause your code to leak file handles and other resources, which would be problematic in a real application with a high volume of requests.

Normally you would handle this by writing something like:

CSVReaderHeaderAware csvReader = null;
try {
  csvReader = ...
  // process the file
} catch (IOException ..) {
  // file doesn't exist (although it could be another error like access denied)
} catch (/* other errors thrown by CSV library */) {
} finally {
  if (csvReader != null) {
    csvReader.Close();
  }
}

Fortunately, modern Java has better syntax for it, but I don't know what version app engine uses. See this article: https://www.infoq.com/news/2010/08/arm-blocks/

try(CSVReaderHeaderAware csvReader = new CSVReaderHeaderAware(new FileReader(filePath))) {
  // process the file
  // no need to close it manually
} catch (IOException /* or whatever */) {
  // ...
}

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I faintly remember covering this in my OOP course in Waterloo and it completely slipped my mind when I was writing this. Thanks for pointing it out, I've opened an issue #58 so that I can look into this and consciously work on it to remember for good.

csvReader.close();
} catch (IOException exception) {
System.out.println("File not found.");
}
}

}