Mailing-List: contact dev-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Commons Developers List" <dev@commons.apache.org>
Received-SPF: pass (nike.apache.org: domain of garydgregory@gmail.com
 designates 209.85.214.178 as permitted sender)
References: <20130315162027.A01A123888E7@eris.apache.org>
 <CAOGo0VbBjvdA6b_63oj2k4Zv8syVyDaXJbxa8V3EWEMeiY0=Uw@mail.gmail.com>
From: Gary Gregory <garydgregory@gmail.com>
Mime-Version: 1.0 (1.0)
In-Reply-To: 
 <CAOGo0VbBjvdA6b_63oj2k4Zv8syVyDaXJbxa8V3EWEMeiY0=Uw@mail.gmail.com>
Date: Mon, 18 Mar 2013 07:08:07 -0400
Message-ID: <8448719218289634295@unknownmsgid>
Subject: Re: svn commit: r1457003 -
 /commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/util/mime/Base64Decoder.java
To: Commons Developers List <dev@commons.apache.org>
Content-Type: text/plain; charset=UTF-8

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