Usage of new API, cleanups (reduced)

src/org/adblockplus/ChunkedOutputStream.java

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2014 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 package org.adblockplus;
 
 import java.io.FilterOutputStream;
 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);
   }
 
   @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);
   }
 }