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 Exception
s
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 aan 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<String>
of names
, since the id
and startDate
are the same for all the names?