The exercise is from a Java-course in which I'm enrolled.
The classes Book and APIResponseParser were provided by the instructor. For Book I had to add just a few getter- and setter-methods.
For APIResponseParser the instance-version of parse() had to be written. The static version of parse() had to be completed. Please see the comments within the code.
These values are then used for to instantiate an object of the class Book. For some values is additional type conversion necessary.
Here's my code:
public class Book {
private String title;
private String author;
private int publicationYear;
private double averageRating;
private int ratingsCount;
private String imageUrl;
// Add getters & setters for author, averageRating, and ratingsCount
public String getTitle() {
return title;
}
public void setTitle(String title) {
this.title = title;
}
public String getAuthor() {
return author;
}
public void setAuthor(String author) {
this.author = author;
}
public int getPublicationYear() {
return publicationYear;
}
public void setPublicationYear(int publicationYear) {
this.publicationYear = publicationYear;
}
public String getImageUrl() {
return imageUrl;
}
public void setImageUrl(String imageUrl) {
this.imageUrl = imageUrl;
}
public void setAverageRating(double rating) {
this.averageRating = rating;
}
public double getAverageRating() {
return averageRating;
}
public void setRatingsCount(int ratingsCount) {
this.ratingsCount = ratingsCount;
}
public int getRatingsCount() {
return ratingsCount;
}
}
// ----- Main Class with the String-parser ---------
public class APIResponseParser {
/**
* Parses the input text and returns a Book instance containing
* the parsed data.
* @param response text to be parsed
* @return Book instance containing parsed data
*/
public static Book parse(String response) {
// -- Start: Provided by instructor --
Book book = new Book();
String endRule = "<";
String startRule = "<title>";
APIResponseParser arp = new APIResponseParser();
String title = arp.parse(response, startRule, endRule);
book.setTitle(title);
// -- End: Provided by instructor --
startRule = "<name>";
String name = arp.parse(response, startRule, endRule);
book.setAuthor(name);
startRule = "<original_publication_year type=\"integer\">";
String publicationYear = arp.parse(response, startRule, endRule);
book.setPublicationYear(new Integer(publicationYear));
startRule = "<average_rating>";
String averageRating = arp.parse(response, startRule, endRule);
book.setAverageRating(new Double(averageRating));
startRule = "<ratings_count type=\"integer\">";
String ratingsCount = arp.parse(response, startRule, endRule);
book.setRatingsCount(new Integer(ratingsCount.replaceAll(",", "")));
startRule = "<image_url>";
String imageUrl = arp.parse(response, startRule, endRule);
book.setImageUrl(imageUrl);
return book;
}
private String parse(String response, String startRule, String endRule) {
int pos = response.indexOf(startRule);
int start = pos + startRule.length();
int end = response.indexOf(endRule, start);
return response.substring(start, end);
}
// write overloaded parse method with the 3 parameters response, startRule, endRule
public static void main(String[] args) {
String response = "<work>" +
"<id type=\"integer\">2361393</id>" +
"<books_count type=\"integer\">813</books_count>" +
"<ratings_count type=\"integer\">1,16,315</ratings_count>" +
"<text_reviews_count type=\"integer\">3439</text_reviews_count>" +
"<original_publication_year type=\"integer\">1854</original_publication_year>" +
"<original_publication_month type=\"integer\" nil=\"true\"/>" +
"<original_publication_day type=\"integer\" nil=\"true\"/>" +
"<average_rating>3.79</average_rating>" +
"<best_book type=\"Book\">" +
"<id type=\"integer\">16902</id>" +
"<title>Walden</title>" +
"<author>" +
"<id type=\"integer\">10264</id>" +
"<name>Henry David Thoreau</name>" +
"</author>" +
"<image_url>" +
"http://images.gr-assets.com/books/1465675526m/16902.jpg" +
"</image_url>" +
"<small_image_url>" +
"http://images.gr-assets.com/books/1465675526s/16902.jpg" +
"</small_image_url>" +
"</best_book>" +
"</work>";
Book book = APIResponseParser.parse(response);
out.println(book.getAuthor());
out.println(book.getTitle());
out.println(book.getAverageRating());
out.println(book.getImageUrl());
}
}
The code has passed the automated tests.
But I would appreciate some real feedback.
What do you think about my string-processing and the way I've done the type conversion? What would you have done different and why?
All comments, hints and answers appreciated.
-
\$\begingroup\$ What version of Java are you using? \$\endgroup\$candied_orange– candied_orange2018年05月27日 13:19:17 +00:00Commented May 27, 2018 at 13:19
-
\$\begingroup\$ It would help if you more clearly specified which part of the code was provided by instructor and which one by you (maybe before and after code snippets?). \$\endgroup\$Łukasz Ciosek– Łukasz Ciosek2018年05月27日 13:35:15 +00:00Commented May 27, 2018 at 13:35
-
\$\begingroup\$ @candied_orange It's version Java 8. \$\endgroup\$michael.zech– michael.zech2018年05月28日 05:57:56 +00:00Commented May 28, 2018 at 5:57
-
\$\begingroup\$ @ŁukaszCiosek Thanks for the hint. I have tried to improve my question accordingly. \$\endgroup\$michael.zech– michael.zech2018年05月28日 06:05:21 +00:00Commented May 28, 2018 at 6:05
1 Answer 1
"Main work was the instance-version of parse()" I am guessing that this means the non static parse()
method. it contains 4 lines of code. out of 140 lines posted, that is very little "Main work"...
like it was said in the comments, it is not clear what code is yours and what was given. So I restricted my comments to the non static parse()
method in APIResponseParser
class:
the
APIResponseParser
class seems (to me) to fit the "utility" pattern of class that has only static methods (likeJava.lang.Objects
or Apache'sStringUtils
). Therefore, I recommend that the non staticparse()
method be changed into static. That would eliminate the calling (static) method having to instantiate an instance ofAPIResponseParser
.so there is already a
parse
method inAPIResponseParser
. For clarity sake, choose a different name for your method (unless it is dictated by the exercise). how aboutgetXmlElementText()
I am not sure what was the exercise but you need to understand that this is not the proper way to parse an XML document into a Java bean. In the 'real world' there are 3rd party parsers that can build a
Book
out of XML document.Now, if we stick to string processing, the method does not really parse an XML element. all it does is cut a substring of a given string. I mean it is fed the starting and ending tokens. If it is to be an XML element parser, it should be given just the element name and it should figure out how to find the text part of the element. in psuedo code:
String getXmlElementText(String elementName) {
search for "<" + elementName;
search for next ">"
start capturing text value;
search for next "</" + elementName -> stop capturing text value.
return text value
}