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 2 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
35 changes: 20 additions & 15 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,7 +14,10 @@

package com.google.sps.servlets;

import com.opencsv.CSVReaderHeaderAware;
import com.google.gson.Gson;

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Is this line break intended?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Nope, it was automatically added when I used a VSCode shortcut. Also, I need to refresh my memory on the style guide. Thanks for catching that, I fixed it on the latest commit!

Copy link
Owner Author

Choose a reason for hiding this comment

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

RESOLVED

import java.io.FileReader;
import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
Expand All @@ -32,21 +35,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 = "/home/tanzachary/step/portfolio/src/main/webapp/WEB-INF/coronavirus-stats-by-country.csv";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe use a relative path instead?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Initially I tried to use a relative path but it wasn't working then I found out that it was coming out from a target folder that maven uses to run the app. Thanks for the suggestion, I've implemented this on the latest commit!

Copy link
Owner Author

Choose a reason for hiding this comment

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

RESOLVED

readCsv(filePath);
}

@Override
Expand All @@ -56,5 +46,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.");
}
}

}