commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@gmail.com>
Subject Re: svn commit: r1457003 - /commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/util/mime/Base64Decoder.java
Date Mon, 18 Mar 2013 11:08:07 GMT
I agree with Sebb.

Gary

On Mar 18, 2013, at 4:35, sebb <sebbaz@gmail.com> wrote:

> On 15 March 2013 16:20,  <simonetripodi@apache.org> wrote:
>> Author: simonetripodi
>> Date: Fri Mar 15 16:20:27 2013
>> New Revision: 1457003
>>
>> URL: http://svn.apache.org/r1457003
>> Log:
>> checkstyle: magic numbers
>
> -1
>
> Although these changes don't affect the code, they are wrong.
>
> Just because the same number (e.g. 3) is used in several places, does
> not mean that it represents the same value.
> For example, 60 is the standard number of seconds in a minute, and
> minutes in an hour, but code needs to use different names for them.
>
> The numbers now replaced by MASK_nBITS fields were used as shift
> counts and index offsets, not masks.
>
> Different constants must be used for each different use, even if the
> values are the same, and the name should reflect the meaning of the
> constant.
>
>> Modified:
>>    commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/util/mime/Base64Decoder.java
>>
>> Modified: commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/util/mime/Base64Decoder.java
>> URL: http://svn.apache.org/viewvc/commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/util/mime/Base64Decoder.java?rev=1457003&r1=1457002&r2=1457003&view=diff
>> ==============================================================================
>> --- commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/util/mime/Base64Decoder.java
(original)
>> +++ commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/util/mime/Base64Decoder.java
Fri Mar 15 16:20:27 2013
>> @@ -25,6 +25,26 @@ import java.io.OutputStream;
>> final class Base64Decoder {
>>
>>     /**
>> +     * Bytes per undecoded block.
>> +     */
>> +    private static final int BYTES_PER_UNENCODED_BLOCK = 3;
>> +
>> +    /**
>> +     * 2 bits mask.
>> +     */
>> +    private static final int MASK_2BITS = 2;
>> +
>> +    /**
>> +     * 4 bits mask.
>> +     */
>> +    private static final int MASK_4BITS = 4;
>> +
>> +    /**
>> +     * 6 bits mask.
>> +     */
>> +    private static final int MASK_6BITS = 6;
>> +
>> +    /**
>>      * Set up the encoding table.
>>      */
>>     private static final byte[] ENCODING_TABLE = {
>> @@ -101,7 +121,7 @@ final class Base64Decoder {
>>         }
>>
>>         int  i = off;
>> -        int  finish = end - 4;
>> +        int  finish = end - MASK_4BITS;
>>
>>         while (i < finish) {
>>             while ((i < finish) && ignore((char) data[i])) {
>> @@ -128,40 +148,40 @@ final class Base64Decoder {
>>
>>             b4 = DECODING_TABLE[data[i++]];
>>
>> -            out.write((b1 << 2) | (b2 >> 4));
>> -            out.write((b2 << 4) | (b3 >> 2));
>> -            out.write((b3 << 6) | b4);
>> +            out.write((b1 << MASK_2BITS) | (b2 >> MASK_4BITS));
>> +            out.write((b2 << MASK_4BITS) | (b3 >> MASK_2BITS));
>> +            out.write((b3 << MASK_6BITS) | b4);
>>
>> -            outLen += 3;
>> +            outLen += BYTES_PER_UNENCODED_BLOCK;
>>         }
>>
>> -        if (data[end - 2] == PADDING) {
>> -            b1 = DECODING_TABLE[data[end - 4]];
>> -            b2 = DECODING_TABLE[data[end - 3]];
>> +        if (data[end - MASK_2BITS] == PADDING) {
>> +            b1 = DECODING_TABLE[data[end - MASK_4BITS]];
>> +            b2 = DECODING_TABLE[data[end - BYTES_PER_UNENCODED_BLOCK]];
>>
>> -            out.write((b1 << 2) | (b2 >> 4));
>> +            out.write((b1 << MASK_2BITS) | (b2 >> MASK_4BITS));
>>
>>             outLen += 1;
>>         } else if (data[end - 1] == PADDING) {
>> -            b1 = DECODING_TABLE[data[end - 4]];
>> -            b2 = DECODING_TABLE[data[end - 3]];
>> -            b3 = DECODING_TABLE[data[end - 2]];
>> +            b1 = DECODING_TABLE[data[end - MASK_4BITS]];
>> +            b2 = DECODING_TABLE[data[end - BYTES_PER_UNENCODED_BLOCK]];
>> +            b3 = DECODING_TABLE[data[end - MASK_2BITS]];
>>
>> -            out.write((b1 << 2) | (b2 >> 4));
>> -            out.write((b2 << 4) | (b3 >> 2));
>> +            out.write((b1 << MASK_2BITS) | (b2 >> MASK_4BITS));
>> +            out.write((b2 << MASK_4BITS) | (b3 >> MASK_2BITS));
>>
>> -            outLen += 2;
>> +            outLen += MASK_2BITS;
>>         } else {
>> -            b1 = DECODING_TABLE[data[end - 4]];
>> -            b2 = DECODING_TABLE[data[end - 3]];
>> -            b3 = DECODING_TABLE[data[end - 2]];
>> +            b1 = DECODING_TABLE[data[end - MASK_4BITS]];
>> +            b2 = DECODING_TABLE[data[end - BYTES_PER_UNENCODED_BLOCK]];
>> +            b3 = DECODING_TABLE[data[end - MASK_2BITS]];
>>             b4 = DECODING_TABLE[data[end - 1]];
>>
>> -            out.write((b1 << 2) | (b2 >> 4));
>> -            out.write((b2 << 4) | (b3 >> 2));
>> -            out.write((b3 << 6) | b4);
>> +            out.write((b1 << MASK_2BITS) | (b2 >> MASK_4BITS));
>> +            out.write((b2 << MASK_4BITS) | (b3 >> MASK_2BITS));
>> +            out.write((b3 << MASK_6BITS) | b4);
>>
>> -            outLen += 3;
>> +            outLen += BYTES_PER_UNENCODED_BLOCK;
>>         }
>>
>>         return outLen;
>
> ---------------------------------------------------------------------
> 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


Mime
View raw message