CSV to Object converter with JavaDoc comments

Question

This is a simple class but I'm looking to check I have the basics down to scratch.

Is my documentation thorough enough? (I'm very new to writing JavaDoc) Have I created too many variables, and does this sacrifice performance for readability in a bad way?

This class is called once to create an array of TestDataObjects to use with our Parsing unit tests.

 /**
 * Converts our test data csv in to an ArrayList
 * We divide the csv entries up line by line for each calendar event then create a TestData object from each line.
 * The fields we wish to parse have negated new line characters to ensure they don't interfere with the csv parsing.
 * Once the csv has been parsed we need to unnegate these so the test data is the same as the original data.
 * @param calendar The csv to convert
 * @return An Array List of TestData objects that have all the three fields we wish to parse and the correct results we wish to obtain.
 */
private ArrayList<TestData> loadTestData(String calendar) {
 ArrayList<TestData> testDataArray = new ArrayList<>();
 String[] lines = calendar.split("\r\n");
 // always skip lines[0] as it contains the headers
 for (int i = 1; i < lines.length; ++i) {
 ArrayList<String> fields = CSVReader.parseLine(lines[i]);
 String title = fields.get(0).replace("\\n", "\n");
 String location = fields.get(1).replace("\\n", "\n");
 String description = fields.get(2).replace("\\n", "\n");
 ArrayList<String> correctHostCodes = CSVReader.parseLine(fields.get(3));
 ArrayList<String> correctParticipantCodes = CSVReader.parseLine(fields.get(4));
 ArrayList<String> correctPhoneNumbers = CSVReader.parseLine(fields.get(5));
 TestData testDataObject = new TestData(title + "\n" + location + "\n" + description, correctHostCodes, correctParticipantCodes, correctPhoneNumbers);
 testDataArray.add(testDataObject);
 }
 return testDataArray;
}

• notionquest: Are you using opencsv for parsing the line i.e. CSVReader.parseLine? Would you like to handle any IOException in this method?
• Declan McKenna: @notionquest My colleague built the CSVReader I think it's based off of this. mkyong.com/java/how-to-read-and-parse-csv-file-in-java
• Declan McKenna: @notionquest As for the IOException, I'm not sure, what would the benefits be to catching it?

Answer 1

OS-dependent line separator

You can use System.lineSeparator() to retrieve the correct OS-dependent line separator String, instead of hard-coding \r\n.

Method parameters

private ArrayList<TestData> loadTestData(String calendar) {
 // ...
}

calendar does not sound like it's supposed to be CSV data. You can consider calling it just data, or events to relate it closer to your use case.

Interfaces over implementations for documentation too

On a related note to @mdfst13's answer, the use of List (or 'list') should be preferred over ArrayList (or 'Array List') for similar reasons.

1. Should the implementation type change in the future, your Javadoc is not rendered outdated.

2. Typically, the implementation classes' names are less 'English'-like than the interfaces, so it's easier to understand lists and maps, instead of 'Array List' and 'Hash Maps'. :)

HTML and Javadoc tags

Be sure to use some simple HTML tags to aid in the eventual layout you see in your IDE/browser. In addition, the javadoc tool itself understands some tags, such as the {@link package.class} that automatically creates a link to the target class.

Knowing your audience

You use a great deal of our/us in the Javadoc, presumably to establish some sort of a rapport with the end-user using this code. There's nothing inherently wrong with this. :)

However, I can't recall any public Javadocs out there written from the first-person perspective, so I can't comment (pun intended) whether this is a bad practice or not.

In any case, I suppose cutting down on such pronouns makes the Javadoc shorter and to-the-point. For example:

/**
 * Converts test data from CSV format to a list.<br>
 * Line-by-line calendar events are converted to individual {@link TestData} objects.<br>
 * Negated new line characters in the fields to parse are un-negated so that the test data
 * is the same as the original.
 *
 * @param events The data to convert, in CSV format.
 * @return A {@link List} of {@link TestData} objects with all fields to parse and the
 * correct results to obtain.
 */

• Declan McKenna: Am I right to describe the TestData within the javadoc @return statement for this method when the user could just click the link to the TestData documentation?
• h.j.k.: It's useful as a direct link within this method's Javadoc, the user may be viewing the no-frames version of the Javadoc. ;)

Answer 2

Prefer interfaces to implementations

private ArrayList<TestData> loadTestData(String calendar) {
 ArrayList<TestData> testDataArray = new ArrayList<>();

and

 ArrayList<String> fields = CSVReader.parseLine(lines[i]);

and

 ArrayList<String> correctHostCodes = CSVReader.parseLine(fields.get(3));
 ArrayList<String> correctParticipantCodes = CSVReader.parseLine(fields.get(4));
 ArrayList<String> correctPhoneNumbers = CSVReader.parseLine(fields.get(5));

You would normally write these as

private List<TestData> loadTestData(String calendar) {
 List<TestData> testDataArray = new ArrayList<>();

and

 List<String> fields = CSVReader.parseLine(lines[i]);

and

 List<String> correctHostCodes = CSVReader.parseLine(fields.get(3));
 List<String> correctParticipantCodes = CSVReader.parseLine(fields.get(4));
 List<String> correctPhoneNumbers = CSVReader.parseLine(fields.get(5));

That way, even if parseLine changes its return type, you don't have to change unless it changes to something that doesn't implement the List interface.

Range-based for loops

 String[] lines = calendar.split("\r\n");
 // always skip lines[0] as it contains the headers
 for (int i = 1; i < lines.length; ++i) {
 ArrayList<String> fields = CSVReader.parseLine(lines[i]);

You could rewrite this as

 List<String> lines = Arrays.asList(calendar.split("\r\n"));
 // always skip lines[0] as it contains the headers
 for (String line : lines.sublist(1, lines.size())) {
 List<String> fields = CSVReader.parseLine(line);

Or not. If you find the indexed form easier to follow, that works too.