Usage of new API, cleanups (reduced)

src/org/adblockplus/ChunkedOutputStream.java

Index: src/org/adblockplus/ChunkedOutputStream.java
diff --git a/src/org/adblockplus/ChunkedOutputStream.java b/src/org/adblockplus/ChunkedOutputStream.java
index f4f26d120ef5c1ca6ba9230fb3f63e868358daf6..92f93ec9e838311507447dcd3234b067a4d86b34 100644
--- a/src/org/adblockplus/ChunkedOutputStream.java
+++ b/src/org/adblockplus/ChunkedOutputStream.java
@@ -22,18 +22,17 @@ import java.io.IOException;
 import java.io.OutputStream;
 
 /**
- * ChunkedOutputStream implements chunked HTTP transfer encoding wrapper for
- * OutputStream.
+ * ChunkedOutputStream implements chunked HTTP transfer encoding wrapper for OutputStream.
  */
 public class ChunkedOutputStream extends FilterOutputStream
 {
-  private static final int MAX_CHUNK_SIZE = 2048;
+  private final static int MAX_CHUNK_SIZE = 2048;
+  private final static byte[] CRLF = { '\r', '\n' };
+  private final static byte[] FINAL_CHUNK = new byte[] { '0', '\r', '\n', '\r', '\n' };
 
-  private static final byte[] CRLF = {'\r', '\n'};
-  private static final byte[] FINAL_CHUNK = new byte[] {'0', '\r', '\n', '\r', '\n'};
   private boolean wroteFinalChunk = false;
 
-  public ChunkedOutputStream(OutputStream out)
+  public ChunkedOutputStream(final OutputStream out)
   {
     super(out);
   }
@@ -41,66 +40,66 @@ public class ChunkedOutputStream extends FilterOutputStream
   @Override
   public void close() throws IOException
   {
-    if (!wroteFinalChunk)
-      writeFinalChunk();
+    // TODO: Huh? No sync, no 'this.wroteFinalChunk = false'?

[Felix Dahlke, 2014/04/16 15:24:25]

Can you make this TODO more clear? If you think this needs to be synchronized,
please say why. And why do you think wroteFinalChunk needs to be set to false?

[René Jeschke, 2014/04/16 17:51:47]

Removed. It will throw an exception anyway if it is called twice.

+    if (!this.wroteFinalChunk)

[Felix Dahlke, 2014/04/16 15:24:25]

I agree to adding final's everywhere, but let's please keep unrelated changes to
a minimum otherwise. Always prefixing instance variables with "this" is an
example of this.

[René Jeschke, 2014/04/16 17:51:47]

Done.

+    {

[Felix Dahlke, 2014/04/16 15:24:25]

This is also an unrelated change.

[René Jeschke, 2014/04/16 17:51:47]

Done.

+      this.writeFinalChunk();
+    }
     super.close();
   }
 
   @Override
-  public void write(byte[] buffer, int offset, int length) throws IOException
+  public void write(final byte[] buffer, final int offset, final int length) throws IOException
   {
-    writeChunk(buffer, offset, length);
+    this.writeChunk(buffer, offset, length);
   }
 
   @Override
-  public void write(byte[] buffer) throws IOException
+  public void write(final byte[] buffer) throws IOException
   {
     int offset = 0;
     int remain = buffer.length;
     while (remain > 0)
     {
-      int size = MAX_CHUNK_SIZE;
-      if (size > remain)
-        size = remain;
-      writeChunk(buffer, offset, size);
+      final int size = Math.min(remain, MAX_CHUNK_SIZE);
+      this.writeChunk(buffer, offset, size);
       offset += size;
       remain -= size;
     }
   }
 
   @Override
-  public void write(int oneByte) throws IOException
+  public void write(final int oneByte) throws IOException
   {
     throw new UnsupportedOperationException("Not implemented");
   }
 
   public void writeFinalChunk() throws IOException
   {
-    out.write(FINAL_CHUNK);
-    out.flush();
-    wroteFinalChunk = true;
+    this.out.write(FINAL_CHUNK);
+    this.out.flush();
+    this.wroteFinalChunk = true;
   }
 
-  private void writeChunk(byte buffer[], int offset, int length) throws IOException
+  private void writeChunk(final byte buffer[], final int offset, final int length) throws IOException
   {
-    // Zero sized buffers are ok on slow connections but not in our case - zero
-    // chunk is used to indicate the end of transfer.
+    // Zero sized buffers are ok on slow connections but not in our case - zero chunk is used to indicate the end of transfer.

[Felix Dahlke, 2014/04/16 15:24:25]

Our rule is to wrap at 100 columns, which is what this does. We're not too
strict about enforcing it, but we should definitely not change code that adheres
to it.

[René Jeschke, 2014/04/16 17:51:47]

Done.

     if (length > 0)
     {
       // Write the chunk length as a hex number
-      writeHex(length);
+      this.writeHex(length);
       // Write the data
-      out.write(buffer, offset, length);
+      this.out.write(buffer, offset, length);
       // Write a CRLF
-      out.write(CRLF);
+      this.out.write(CRLF);
       // Flush the underlying stream
-      out.flush();
+      this.out.flush();
     }
   }
 
-  private void writeHex(int i) throws IOException
+  private void writeHex(final int i) throws IOException
   {
-    out.write(Integer.toHexString(i).getBytes());
-    out.write(CRLF);
+    this.out.write(Integer.toHexString(i).getBytes());
+    this.out.write(CRLF);
   }
 }