You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by si...@apache.org on 2013/03/15 11:37:01 UTC
svn commit: r1456871 -
/commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/util/mime/Base64Decoder.java
Author: simonetripodi
Date: Fri Mar 15 10:37:01 2013
New Revision: 1456871
URL: http://svn.apache.org/r1456871
Log:
checkstyle: '256' is a magic number.
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=1456871&r1=1456870&r2=1456871&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 10:37:01 2013
@@ -48,9 +48,14 @@ final class Base64Decoder {
private static final byte PADDING = (byte) '=';
/**
+ * the decoding table size.
+ */
+ private static final int DECODING_TABLE_SIZE = 256;
+
+ /**
* set up the decoding table.
*/
- private static final byte[] DECODING_TABLE = new byte[256];
+ private static final byte[] DECODING_TABLE = new byte[DECODING_TABLE_SIZE];
static {
for (int i = 0; i < ENCODING_TABLE.length; i++) {
Re: svn commit: r1456871 - /commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/util/mime/Base64Decoder.java
Posted by sebb <se...@gmail.com>.
On 17 March 2013 13:18, Simone Tripodi <si...@apache.org> wrote:
>>> /**
>>> + * the decoding table size.
>>> + */
>>> + private static final int DECODING_TABLE_SIZE = 256;
>>> +
>>
>> The Javadoc does not say why the value 256 is used, so the number is
>> still a magic number ...
>>
>
> What should be added more than `the decoding table size` in javadoc?
Why is it 256 and not 127 or 13 or 512 or some other arbitrary number?
Are there any restrictions on what it could be?
About the only numbers that are not always magic are 0, 1 and -1.
Other numbers need to be explained.
> I'd rather continue investing time on a private static field of a
> package-private class...
>
>>
>> In this case, I'm not sure that using a constant add any benefit as
>> the value was only used once.
>> I think Checkstyle is being too strict here; I suggest leaving this
>> change, but I don't see a need to fix everything the Checkstyle does
>> not like.
>>
>
> Agreed - I had two options:
>
> 1) suppress the rule;
>
> 2) just make checkstyle happy.
>
> Even if I was really tempted by #1, at the end I went to #2 to be
> conform to more popular way of coding - I sure someone would have
> fixed it, sooner or later...
Unfortunately the fix means that the problem - that the number is
magic - is no longer reported, so the whole point of the Checkstyle
rule has been lost.
> 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: r1456871 - /commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/util/mime/Base64Decoder.java
Posted by Simone Tripodi <si...@apache.org>.
>> /**
>> + * the decoding table size.
>> + */
>> + private static final int DECODING_TABLE_SIZE = 256;
>> +
>
> The Javadoc does not say why the value 256 is used, so the number is
> still a magic number ...
>
What should be added more than `the decoding table size` in javadoc?
I'd rather continue investing time on a private static field of a
package-private class...
>
> In this case, I'm not sure that using a constant add any benefit as
> the value was only used once.
> I think Checkstyle is being too strict here; I suggest leaving this
> change, but I don't see a need to fix everything the Checkstyle does
> not like.
>
Agreed - I had two options:
1) suppress the rule;
2) just make checkstyle happy.
Even if I was really tempted by #1, at the end I went to #2 to be
conform to more popular way of coding - I sure someone would have
fixed it, sooner or later...
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
Re: svn commit: r1456871 - /commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/util/mime/Base64Decoder.java
Posted by sebb <se...@gmail.com>.
On 15 March 2013 10:37, <si...@apache.org> wrote:
> Author: simonetripodi
> Date: Fri Mar 15 10:37:01 2013
> New Revision: 1456871
>
> URL: http://svn.apache.org/r1456871
> Log:
> checkstyle: '256' is a magic number.
>
> 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=1456871&r1=1456870&r2=1456871&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 10:37:01 2013
> @@ -48,9 +48,14 @@ final class Base64Decoder {
> private static final byte PADDING = (byte) '=';
>
> /**
> + * the decoding table size.
> + */
> + private static final int DECODING_TABLE_SIZE = 256;
> +
The Javadoc does not say why the value 256 is used, so the number is
still a magic number ...
> + /**
> * set up the decoding table.
> */
> - private static final byte[] DECODING_TABLE = new byte[256];
> + private static final byte[] DECODING_TABLE = new byte[DECODING_TABLE_SIZE];
In this case, I'm not sure that using a constant add any benefit as
the value was only used once.
I think Checkstyle is being too strict here; I suggest leaving this
change, but I don't see a need to fix everything the Checkstyle does
not like.
> static {
> for (int i = 0; i < ENCODING_TABLE.length; i++) {
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org