I agree with Sebb. Gary
On Mar 18, 2013, at 4:35, 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
