CODEWITHSUNDEEP
javafor-loopsonarqube
Jin Kwon
Jin Kwon

Reputation: 22027

How can I solve a SonarQube complainer in for-loop?

I wrote a method copies bytes from an InputStream to an OutputStream.

// copies bytes from given input stream to specified output stream
// returns the number of bytes copied.
private static long copy(final InputStream input,
                         final OutputStream output)
        throws IOException {
    long count = 0L;
    final byte[] buffer = new byte[4096];
    for (int length; (length = input.read(buffer)) != -1; count += length) {
        output.write(buffer, 0, length);
    }
    return count;
}

And SonarQube complains.

This loop's stop condition tests "length, input, buffer" but the incrementer updates "count".

It is almost always an error when a for loop's stop condition and incrementer don't act on the same variable. Even when it is not, it could confuse future maintainers of the code, and should be avoided.

Is there any better code for the same purpose?

Update

As answers recommended, I did like this and the problem's gone.

// copies bytes from given input stream to specified output stream
// returns the number of bytes copied.
private static long copy(final InputStream input,
                         final OutputStream output)
        throws IOException {
    long count = 0L;
    final byte[] buffer = new byte[4096];
    for (int length; (length = input.read(buffer)) != -1;) {
        output.write(buffer, 0, length);
        count += length;
    }
    return count;
}

Upvotes: 3

Views: 1877

Answers (2)

Tunaki
Tunaki

Reputation: 137269

You are abusing the for loop, that's why SonarQube raises a warning. In the following loop, you are incrementing count in the update clause but the stop condition does not depend on count

for (int length; (length = input.read(buffer)) != -1; count += length) {
                 ^---------------------------------^  ^-------------^
                      does not depend on count        increments count

Instead, you should use a while loop and increment the count inside the loop body:

private static long copy(final InputStream input, final OutputStream output) throws IOException {
    long count = 0;
    final byte[] buffer = new byte[4096];
    int length;
    while ((length = input.read(buffer)) != -1) {
        output.write(buffer, 0, length);
        count += length;
    }
    return count;
}

Upvotes: 6

softarn
softarn

Reputation: 5497

Put the count += length in the for loop body instead. It isn't an incrementer and therefor doesn't belong there, it just keeps count of the copy size.

Upvotes: 1

Related Questions