Group lines from the CSV file

I have a CSV file which looks like this:

ID | Name | Date
"1","Foo1","2013-08-03 21:00:08"
"1","Foo2","2013-08-03 21:00:08"
"2","Foo3","2013-08-03 21:00:08"
...


I need to write a method which returns a list of the lines with same ID and date, for reading the CSV file I am using OpenCSV. My question is whether or not this method can be rewritten in more elegant way.

private String[] currentWorkerLine;
private String relationIdentifier;
private String currentStartDate;
private int rowCount;

private List getWorkers(CSVReader reader) throws Exception{
List workers = new ArrayList<>();
if (currentWorkerLine == null && rowCount == 0){
currentWorkerLine = reader.readNext();
relationIdentifier = currentWorkerLine[0];
currentStartDate = currentWorkerLine[2];
workers.add(currentWorkerLine);
rowCount++;
}

while ((currentWorkerline = reader.readNext()) != null) {
if (!isRelatedLine(currentWorkerline[0]) || !isCurrentDateLine(currentWorkerline[2])){
lastLine = currentWorkerline;
return workers;
}
relationIdentifier = currentWorkerline[0];
currentStartDate = currentWorkerline[2];
workers.add(currentWorkerline);
lastLine = currentWorkerline;
rowCount++;
}
if (currentWorkerline == null && lastLine != null) {
workers.add(lastLine);
rowCount++;
lastLine = null;
}
return workers;
}

private boolean isRelatedLine(String rel) {
return rel.equals(relationIdentifier);
}

private boolean isCurrentDateLine(String date) {
return date.equals(currentStartDate);
}


Class variables

Considering that neither the method nor the list of variables (plus what seems to be a missing private String[] lastLine) are static, I presume these belong to a class which requires instantiation for it to work.

This approach is fine - without knowing what else the class is doing - but I'm not too sure if currentWorkerLine (you have a currentWorkerline too, I'm going to assume that's just a small typo) and lastLine really needs to be 'saved' as class variables. For example, at the end of the method, is there a need to set lastLine = null?

Throwing Exceptions

The only checked Exception I can see from the method is just reader.readNext(), so you may want to replace the throws Exception part with the stricter throws IOException. This forces the method callers to explicitly deal with an I/O-related error, instead of an unusually wide Exception, which offers no (direct) hint as to what has gone wrong.

BTW, on a related note, if the code happens to deal with a malformed CSV input, the line currentWorkerLine = reader.readNext(); will cause NullPointerException should reader.readNext() return null. You may want to check for this, just to be sure.

Comparisons

While your code block is looping through the lines that have passed the identity check, there is no need to keep re-assigning relationIdentifier and currentStartDate:

while ((currentWorkerline = reader.readNext()) != null) {
    if (!isRelatedLine(currentWorkerline[0]) || !isCurrentDateLine(currentWorkerline[2])){
        lastLine = currentWorkerline;
        return workers;
    }
    // they will be the same as the past values, so why re-assign?
    //relationIdentifier = currentWorkerline[0];
    //currentStartDate = currentWorkerline[2];
    workers.add(currentWorkerline);
    lastLine = currentWorkerline;
    rowCount++;
}

Model classes

You may also want to consider having a suitable model class to give a better representation to the three-column row inputs you get over the current String[]. With a model class, you can have a custom equals() implementation that only compares the ID and date values between two 'instances' aka rows, so that the next different row will return false for the comparison. This may simplify not only your approach within the method here, but outside usage as well for the method callers to deal with the List of results.

Also, food for thought, would it be better/more efficient to return one Result object that contains an id, startDate and a List of names, since the id and startDate are the same for all the names?

StackExchange Code Review Q#105922, answer score: 3