You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by sebb <se...@gmail.com> on 2013/03/17 12:00:04 UTC
Re: svn commit: r1456857 - /commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/util/mime/QuotedPrintableDecoder.java
On 15 March 2013 10:11, <si...@apache.org> wrote:
> Author: simonetripodi
> Date: Fri Mar 15 10:11:09 2013
> New Revision: 1456857
>
> URL: http://svn.apache.org/r1456857
> Log:
> checkstyle: '4' is a magic number.
>
> Modified:
> commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/util/mime/QuotedPrintableDecoder.java
>
> Modified: commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/util/mime/QuotedPrintableDecoder.java
> URL: http://svn.apache.org/viewvc/commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/util/mime/QuotedPrintableDecoder.java?rev=1456857&r1=1456856&r2=1456857&view=diff
> ==============================================================================
> --- commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/util/mime/QuotedPrintableDecoder.java (original)
> +++ commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/util/mime/QuotedPrintableDecoder.java Fri Mar 15 10:11:09 2013
> @@ -34,6 +34,11 @@ final class QuotedPrintableDecoder {
> };
>
> /**
> + * The default number of byte shift for decode.
> + */
> + private static final int OUT_SHIFT = 4;
> +
The Javadoc needs to say why the value 4 is used, otherwise it's still
a magic number.
> + /**
> * the decoding table size.
> */
> private static final int DECODING_TABLE_SIZE = 128;
> @@ -99,7 +104,7 @@ final class QuotedPrintableDecoder {
> // this is a hex pair we need to convert back to a single byte.
> byte c1 = DECODING_TABLE[b1];
> byte c2 = DECODING_TABLE[b2];
> - out.write((c1 << 4) | c2);
> + out.write((c1 << OUT_SHIFT) | c2);
> // 3 bytes in, one byte out
> bytesWritten++;
> }
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1456857 - /commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/util/mime/QuotedPrintableDecoder.java
Posted by sebb <se...@gmail.com>.
On 17 March 2013 13:19, Simone Tripodi <si...@apache.org> wrote:
>>> /**
>>> + * The default number of byte shift for decode.
[Actually, it's not a default - the value is fixed, and cannot be
changed without breaking the code.]
>>> + */
>>> + private static final int OUT_SHIFT = 4;
>>> +
>>
>> The Javadoc needs to say why the value 4 is used, otherwise it's still
>> a magic number.
>>
>
> it is even part of black magic if you take in consideration that is it
> a private field of a package-protected class used for internal-use
> only
The point is that the code needs to be documented to make it
understandable for future maintainers.
Why is the output shifted by 4 bits?
It seems to be because it is half a byte, so we should use something
like Byte.SIZE / 2 to define the constant.
Committed in r1457681
> http://people.apache.org/~simonetripodi/
> http://simonetripodi.livejournal.com/
> http://twitter.com/simonetripodi
> http://www.99soft.org/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1456857 - /commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/util/mime/QuotedPrintableDecoder.java
Posted by Simone Tripodi <si...@apache.org>.
>> /**
>> + * The default number of byte shift for decode.
>> + */
>> + private static final int OUT_SHIFT = 4;
>> +
>
> The Javadoc needs to say why the value 4 is used, otherwise it's still
> a magic number.
>
it is even part of black magic if you take in consideration that is it
a private field of a package-protected class used for internal-use
only
http://people.apache.org/~simonetripodi/
http://simonetripodi.livejournal.com/
http://twitter.com/simonetripodi
http://www.99soft.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org