Skip to main content
added 1 character in body
Source Link
h.j.k.
  • 19.2k
  • 3
  • 36
  • 93

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 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?

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 a 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?

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

deleted 5 characters in body
Source Link
h.j.k.
  • 19.2k
  • 3
  • 36
  • 93

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 name, 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 a 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?

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 name, 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 a 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?

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 a 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?

Source Link
h.j.k.
  • 19.2k
  • 3
  • 36
  • 93

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 name, 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 a 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?