FSchumacher commented on code in PR #6694:
URL: https://github.com/apache/jmeter/pull/6694#discussion_r3111217456
##########
src/functions/src/main/java/org/apache/jmeter/functions/FileRowColContainer.java:
##########
@@ -119,13 +120,18 @@ public String getColumn(int row, int col) throws
IndexOutOfBoundsException {
*
*/
public int nextRow() {
- int row = nextRow;
- nextRow++;
- if (nextRow >= fileData.size()) {// 0-based
- nextRow = 0;
+ int size = fileData.size();
+ if (size <= 0) {
+ throw new IllegalStateException("CSV file has no data rows");
+ }
+ while (true) {
+ int current = nextRow.get();
+ int next = (current + 1) % size;
Review Comment:
Theoretical we could still run into an overflow and in that case, we would
get wrong values ;)
And probably more to the point: the name nextRow is misleading. It should
probably be nextRowToReturn or such, as it contains the number, the function
should return and the function sets it to the value the next user should read.
Therefore I think the loop is not that badly written.
If we want to take the AtomicLong-road, we should probably start with a
value of `-1` instead of `0`, so that the first row is still read. But in that
case we would have to check all uses of the value of nextRow outside of the
changed function.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]