I took care about it reverting the MagicNumber constants, see r1457865 http://people.apache.org/~simonetripodi/ http://simonetripodi.livejournal.com/ http://twitter.com/simonetripodi http://www.99soft.org/
On Mon, Mar 18, 2013 at 12:12 PM, sebb <seb...@gmail.com> wrote: > On 18 March 2013 10:45, Simone Tripodi <simonetrip...@apache.org> wrote: >> feel free to adjust them by yourself and make checkstyle silent about >> that rules - I am not so worried as you are about internal-use only >> classes which are intended to be used by fileupload and are not part >> of the APIs. > > See my other post - it's still important that internal code > documentation is accurate. > > Misleading constants should be avoided. > > I will try and fix the code. > > Please don't hide any further magic numbers just to make Checkstyle happy. > Ideally leave the warning in place for someone to fix. > >> http://people.apache.org/~simonetripodi/ >> http://simonetripodi.livejournal.com/ >> http://twitter.com/simonetripodi >> http://www.99soft.org/ >> >> >> On Mon, Mar 18, 2013 at 9:34 AM, sebb <seb...@gmail.com> wrote: >>> On 15 March 2013 16:20, <simonetrip...@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-unsubscr...@commons.apache.org >>> For additional commands, e-mail: dev-h...@commons.apache.org >>> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> For additional commands, e-mail: dev-h...@commons.apache.org >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
