You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by mc...@apache.org on 2013/11/23 12:11:28 UTC
svn commit: r1544770 -
/commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java
Author: mcucchiara
Date: Sat Nov 23 11:11:28 2013
New Revision: 1544770
URL: http://svn.apache.org/r1544770
Log:
Minor code optimizations:
* removed boxing
* removed unused variables
* removed useless casts* removed boxing
* removed unused variables
* removed useless casts
* replaced asArrayList method with Arrays.asList
Modified:
commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java
Modified: commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java
URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java?rev=1544770&r1=1544769&r2=1544770&view=diff
==============================================================================
--- commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java (original)
+++ commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java Sat Nov 23 11:11:28 2013
@@ -16,31 +16,20 @@
*/
package org.apache.commons.lang3.reflect;
-import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-import static org.junit.Assume.assumeNotNull;
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.lang3.reflect.testbed.*;
+import org.junit.Before;
+import org.junit.Test;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.List;
-import org.apache.commons.lang3.ArrayUtils;
-import org.apache.commons.lang3.reflect.testbed.Ambig;
-import org.apache.commons.lang3.reflect.testbed.Foo;
-import org.apache.commons.lang3.reflect.testbed.PrivatelyShadowedChild;
-import org.apache.commons.lang3.reflect.testbed.PublicChild;
-import org.apache.commons.lang3.reflect.testbed.PubliclyShadowedChild;
-import org.apache.commons.lang3.reflect.testbed.StaticContainer;
-import org.apache.commons.lang3.reflect.testbed.StaticContainerChild;
-import org.junit.Before;
-import org.junit.Test;
+import static org.junit.Assert.*;
+import static org.junit.Assume.assumeNotNull;
/**
* Unit tests FieldUtils
@@ -49,12 +38,10 @@ import org.junit.Test;
*/
public class FieldUtilsTest {
- static final String S = "s";
- static final String SS = "ss";
- static final Integer I0 = Integer.valueOf(0);
- static final Integer I1 = Integer.valueOf(1);
- static final Double D0 = Double.valueOf(0.0);
- static final Double D1 = Double.valueOf(1.0);
+ static final Integer I0 = 0;
+ static final Integer I1 = 1;
+ static final Double D0 = 0.0;
+ static final Double D1 = 1.0;
private PublicChild publicChild;
private PubliclyShadowedChild publiclyShadowedChild;
@@ -167,20 +154,12 @@ public class FieldUtilsTest {
assertEquals(5, FieldUtils.getAllFields(PublicChild.class).length);
}
- private <T> List<T> asArrayList(T... values) {
- final ArrayList<T> arrayList = new ArrayList<T>();
- for (T t : values) {
- arrayList.add(t);
- }
- return arrayList;
- }
-
@Test
public void testGetAllFieldsList() {
assertEquals(0, FieldUtils.getAllFieldsList(Object.class).size());
- final List<Field> fieldsNumber = asArrayList(Number.class.getDeclaredFields());
+ final List<Field> fieldsNumber = Arrays.asList(Number.class.getDeclaredFields());
assertEquals(fieldsNumber, FieldUtils.getAllFieldsList(Number.class));
- final List<Field> fieldsInteger = asArrayList(Integer.class.getDeclaredFields());
+ final List<Field> fieldsInteger = Arrays.asList(Integer.class.getDeclaredFields());
final List<Field> allFieldsInteger = new ArrayList<Field>(fieldsInteger);
allFieldsInteger.addAll(fieldsNumber);
assertEquals(allFieldsInteger, FieldUtils.getAllFieldsList(Integer.class));
@@ -462,7 +441,7 @@ public class FieldUtilsTest {
assertEquals(D0, FieldUtils.readField(parentD, privatelyShadowedChild));
try {
- FieldUtils.readField((Field) null, publicChild);
+ FieldUtils.readField(null, publicChild);
fail("a null field should cause an IllegalArgumentException");
} catch (final IllegalArgumentException e) {
// expected
@@ -493,7 +472,7 @@ public class FieldUtilsTest {
assertEquals(D0, FieldUtils.readField(parentD, privatelyShadowedChild, true));
try {
- FieldUtils.readField((Field) null, publicChild, true);
+ FieldUtils.readField(null, publicChild, true);
fail("a null field should cause an IllegalArgumentException");
} catch (final IllegalArgumentException e) {
// expected
@@ -643,7 +622,7 @@ public class FieldUtilsTest {
}
try {
- FieldUtils.readDeclaredField((Object) null, "none");
+ FieldUtils.readDeclaredField(null, "none");
fail("a null target should cause an IllegalArgumentException");
} catch (final IllegalArgumentException e) {
// expected
@@ -727,7 +706,7 @@ public class FieldUtilsTest {
}
try {
- FieldUtils.readDeclaredField((Object) null, "none", true);
+ FieldUtils.readDeclaredField(null, "none", true);
fail("a null target should cause an IllegalArgumentException");
} catch (final IllegalArgumentException e) {
// expected
@@ -839,28 +818,28 @@ public class FieldUtilsTest {
assertEquals("new", StaticContainer.getMutablePrivate());
field = StaticContainer.class.getDeclaredField("IMMUTABLE_PUBLIC");
try {
- FieldUtils.writeStaticField(field, "new", true);
+ FieldUtils.writeStaticField(field, "new", true);
fail("Expected IllegalAccessException");
} catch (final IllegalAccessException e) {
// pass
}
field = StaticContainer.class.getDeclaredField("IMMUTABLE_PROTECTED");
try {
- FieldUtils.writeStaticField(field, "new", true);
+ FieldUtils.writeStaticField(field, "new", true);
fail("Expected IllegalAccessException");
} catch (final IllegalAccessException e) {
// pass
}
field = StaticContainer.class.getDeclaredField("IMMUTABLE_PACKAGE");
try {
- FieldUtils.writeStaticField(field, "new", true);
+ FieldUtils.writeStaticField(field, "new", true);
fail("Expected IllegalAccessException");
} catch (final IllegalAccessException e) {
// pass
}
field = StaticContainer.class.getDeclaredField("IMMUTABLE_PRIVATE");
try {
- FieldUtils.writeStaticField(field, "new", true);
+ FieldUtils.writeStaticField(field, "new", true);
fail("Expected IllegalAccessException");
} catch (final IllegalAccessException e) {
// pass
@@ -1049,13 +1028,13 @@ public class FieldUtilsTest {
}
field = parentClass.getDeclaredField("i");
try {
- FieldUtils.writeField(field, publicChild, Integer.valueOf(Integer.MAX_VALUE));
+ FieldUtils.writeField(field, publicChild, Integer.MAX_VALUE);
} catch (final IllegalAccessException e) {
// pass
}
field = parentClass.getDeclaredField("d");
try {
- FieldUtils.writeField(field, publicChild, Double.valueOf(Double.MAX_VALUE));
+ FieldUtils.writeField(field, publicChild, Double.MAX_VALUE);
} catch (final IllegalAccessException e) {
// pass
}
@@ -1070,11 +1049,11 @@ public class FieldUtilsTest {
FieldUtils.writeField(field, publicChild, Boolean.TRUE, true);
assertEquals(Boolean.TRUE, field.get(publicChild));
field = parentClass.getDeclaredField("i");
- FieldUtils.writeField(field, publicChild, Integer.valueOf(Integer.MAX_VALUE), true);
- assertEquals(Integer.valueOf(Integer.MAX_VALUE), field.get(publicChild));
+ FieldUtils.writeField(field, publicChild, Integer.MAX_VALUE, true);
+ assertEquals(Integer.MAX_VALUE, field.get(publicChild));
field = parentClass.getDeclaredField("d");
- FieldUtils.writeField(field, publicChild, Double.valueOf(Double.MAX_VALUE), true);
- assertEquals(Double.valueOf(Double.MAX_VALUE), field.get(publicChild));
+ FieldUtils.writeField(field, publicChild, Double.MAX_VALUE, true);
+ assertEquals(Double.MAX_VALUE, field.get(publicChild));
}
@Test
@@ -1088,13 +1067,13 @@ public class FieldUtilsTest {
// pass
}
try {
- FieldUtils.writeField(publicChild, "i", Integer.valueOf(1));
+ FieldUtils.writeField(publicChild, "i", 1);
fail("Expected IllegalArgumentException");
} catch (final IllegalArgumentException e) {
// pass
}
try {
- FieldUtils.writeField(publicChild, "d", Double.valueOf(1.0));
+ FieldUtils.writeField(publicChild, "d", 1.0);
fail("Expected IllegalArgumentException");
} catch (final IllegalArgumentException e) {
// pass
@@ -1104,10 +1083,10 @@ public class FieldUtilsTest {
assertEquals("S", FieldUtils.readField(publiclyShadowedChild, "s"));
FieldUtils.writeField(publiclyShadowedChild, "b", Boolean.FALSE);
assertEquals(Boolean.FALSE, FieldUtils.readField(publiclyShadowedChild, "b"));
- FieldUtils.writeField(publiclyShadowedChild, "i", Integer.valueOf(0));
- assertEquals(Integer.valueOf(0), FieldUtils.readField(publiclyShadowedChild, "i"));
- FieldUtils.writeField(publiclyShadowedChild, "d", Double.valueOf(0.0));
- assertEquals(Double.valueOf(0.0), FieldUtils.readField(publiclyShadowedChild, "d"));
+ FieldUtils.writeField(publiclyShadowedChild, "i", 0);
+ assertEquals(0, FieldUtils.readField(publiclyShadowedChild, "i"));
+ FieldUtils.writeField(publiclyShadowedChild, "d", 0.0);
+ assertEquals(0.0, FieldUtils.readField(publiclyShadowedChild, "d"));
FieldUtils.writeField(privatelyShadowedChild, "s", "S");
assertEquals("S", FieldUtils.readField(privatelyShadowedChild, "s"));
@@ -1118,13 +1097,13 @@ public class FieldUtilsTest {
// pass
}
try {
- FieldUtils.writeField(privatelyShadowedChild, "i", Integer.valueOf(1));
+ FieldUtils.writeField(privatelyShadowedChild, "i", 1);
fail("Expected IllegalArgumentException");
} catch (final IllegalArgumentException e) {
// pass
}
try {
- FieldUtils.writeField(privatelyShadowedChild, "d", Double.valueOf(1.0));
+ FieldUtils.writeField(privatelyShadowedChild, "d", 1.0);
fail("Expected IllegalArgumentException");
} catch (final IllegalArgumentException e) {
// pass
@@ -1137,28 +1116,28 @@ public class FieldUtilsTest {
assertEquals("S", FieldUtils.readField(publicChild, "s", true));
FieldUtils.writeField(publicChild, "b", Boolean.TRUE, true);
assertEquals(Boolean.TRUE, FieldUtils.readField(publicChild, "b", true));
- FieldUtils.writeField(publicChild, "i", Integer.valueOf(1), true);
- assertEquals(Integer.valueOf(1), FieldUtils.readField(publicChild, "i", true));
- FieldUtils.writeField(publicChild, "d", Double.valueOf(1.0), true);
- assertEquals(Double.valueOf(1.0), FieldUtils.readField(publicChild, "d", true));
+ FieldUtils.writeField(publicChild, "i", 1, true);
+ assertEquals(1, FieldUtils.readField(publicChild, "i", true));
+ FieldUtils.writeField(publicChild, "d", 1.0, true);
+ assertEquals(1.0, FieldUtils.readField(publicChild, "d", true));
FieldUtils.writeField(publiclyShadowedChild, "s", "S", true);
assertEquals("S", FieldUtils.readField(publiclyShadowedChild, "s", true));
FieldUtils.writeField(publiclyShadowedChild, "b", Boolean.FALSE, true);
assertEquals(Boolean.FALSE, FieldUtils.readField(publiclyShadowedChild, "b", true));
- FieldUtils.writeField(publiclyShadowedChild, "i", Integer.valueOf(0), true);
- assertEquals(Integer.valueOf(0), FieldUtils.readField(publiclyShadowedChild, "i", true));
- FieldUtils.writeField(publiclyShadowedChild, "d", Double.valueOf(0.0), true);
- assertEquals(Double.valueOf(0.0), FieldUtils.readField(publiclyShadowedChild, "d", true));
+ FieldUtils.writeField(publiclyShadowedChild, "i", 0, true);
+ assertEquals(0, FieldUtils.readField(publiclyShadowedChild, "i", true));
+ FieldUtils.writeField(publiclyShadowedChild, "d", 0.0, true);
+ assertEquals(0.0, FieldUtils.readField(publiclyShadowedChild, "d", true));
FieldUtils.writeField(privatelyShadowedChild, "s", "S", true);
assertEquals("S", FieldUtils.readField(privatelyShadowedChild, "s", true));
FieldUtils.writeField(privatelyShadowedChild, "b", Boolean.FALSE, true);
assertEquals(Boolean.FALSE, FieldUtils.readField(privatelyShadowedChild, "b", true));
- FieldUtils.writeField(privatelyShadowedChild, "i", Integer.valueOf(0), true);
- assertEquals(Integer.valueOf(0), FieldUtils.readField(privatelyShadowedChild, "i", true));
- FieldUtils.writeField(privatelyShadowedChild, "d", Double.valueOf(0.0), true);
- assertEquals(Double.valueOf(0.0), FieldUtils.readField(privatelyShadowedChild, "d", true));
+ FieldUtils.writeField(privatelyShadowedChild, "i", 0, true);
+ assertEquals(0, FieldUtils.readField(privatelyShadowedChild, "i", true));
+ FieldUtils.writeField(privatelyShadowedChild, "d", 0.0, true);
+ assertEquals(0.0, FieldUtils.readField(privatelyShadowedChild, "d", true));
}
@Test
@@ -1176,13 +1155,13 @@ public class FieldUtilsTest {
// pass
}
try {
- FieldUtils.writeDeclaredField(publicChild, "i", Integer.valueOf(1));
+ FieldUtils.writeDeclaredField(publicChild, "i", 1);
fail("Expected IllegalArgumentException");
} catch (final IllegalArgumentException e) {
// pass
}
try {
- FieldUtils.writeDeclaredField(publicChild, "d", Double.valueOf(1.0));
+ FieldUtils.writeDeclaredField(publicChild, "d", 1.0);
fail("Expected IllegalArgumentException");
} catch (final IllegalArgumentException e) {
// pass
@@ -1192,10 +1171,10 @@ public class FieldUtilsTest {
assertEquals("S", FieldUtils.readDeclaredField(publiclyShadowedChild, "s"));
FieldUtils.writeDeclaredField(publiclyShadowedChild, "b", Boolean.FALSE);
assertEquals(Boolean.FALSE, FieldUtils.readDeclaredField(publiclyShadowedChild, "b"));
- FieldUtils.writeDeclaredField(publiclyShadowedChild, "i", Integer.valueOf(0));
- assertEquals(Integer.valueOf(0), FieldUtils.readDeclaredField(publiclyShadowedChild, "i"));
- FieldUtils.writeDeclaredField(publiclyShadowedChild, "d", Double.valueOf(0.0));
- assertEquals(Double.valueOf(0.0), FieldUtils.readDeclaredField(publiclyShadowedChild, "d"));
+ FieldUtils.writeDeclaredField(publiclyShadowedChild, "i", 0);
+ assertEquals(0, FieldUtils.readDeclaredField(publiclyShadowedChild, "i"));
+ FieldUtils.writeDeclaredField(publiclyShadowedChild, "d", 0.0);
+ assertEquals(0.0, FieldUtils.readDeclaredField(publiclyShadowedChild, "d"));
try {
FieldUtils.writeDeclaredField(privatelyShadowedChild, "s", "S");
@@ -1210,13 +1189,13 @@ public class FieldUtilsTest {
// pass
}
try {
- FieldUtils.writeDeclaredField(privatelyShadowedChild, "i", Integer.valueOf(1));
+ FieldUtils.writeDeclaredField(privatelyShadowedChild, "i", 1);
fail("Expected IllegalArgumentException");
} catch (final IllegalArgumentException e) {
// pass
}
try {
- FieldUtils.writeDeclaredField(privatelyShadowedChild, "d", Double.valueOf(1.0));
+ FieldUtils.writeDeclaredField(privatelyShadowedChild, "d", 1.0);
fail("Expected IllegalArgumentException");
} catch (final IllegalArgumentException e) {
// pass
@@ -1238,13 +1217,13 @@ public class FieldUtilsTest {
// pass
}
try {
- FieldUtils.writeDeclaredField(publicChild, "i", Integer.valueOf(1), true);
+ FieldUtils.writeDeclaredField(publicChild, "i", 1, true);
fail("Expected IllegalArgumentException");
} catch (final IllegalArgumentException e) {
// pass
}
try {
- FieldUtils.writeDeclaredField(publicChild, "d", Double.valueOf(1.0), true);
+ FieldUtils.writeDeclaredField(publicChild, "d", 1.0, true);
fail("Expected IllegalArgumentException");
} catch (final IllegalArgumentException e) {
// pass
@@ -1254,19 +1233,19 @@ public class FieldUtilsTest {
assertEquals("S", FieldUtils.readDeclaredField(publiclyShadowedChild, "s", true));
FieldUtils.writeDeclaredField(publiclyShadowedChild, "b", Boolean.FALSE, true);
assertEquals(Boolean.FALSE, FieldUtils.readDeclaredField(publiclyShadowedChild, "b", true));
- FieldUtils.writeDeclaredField(publiclyShadowedChild, "i", Integer.valueOf(0), true);
- assertEquals(Integer.valueOf(0), FieldUtils.readDeclaredField(publiclyShadowedChild, "i", true));
- FieldUtils.writeDeclaredField(publiclyShadowedChild, "d", Double.valueOf(0.0), true);
- assertEquals(Double.valueOf(0.0), FieldUtils.readDeclaredField(publiclyShadowedChild, "d", true));
+ FieldUtils.writeDeclaredField(publiclyShadowedChild, "i", 0, true);
+ assertEquals(0, FieldUtils.readDeclaredField(publiclyShadowedChild, "i", true));
+ FieldUtils.writeDeclaredField(publiclyShadowedChild, "d", 0.0, true);
+ assertEquals(0.0, FieldUtils.readDeclaredField(publiclyShadowedChild, "d", true));
FieldUtils.writeDeclaredField(privatelyShadowedChild, "s", "S", true);
assertEquals("S", FieldUtils.readDeclaredField(privatelyShadowedChild, "s", true));
FieldUtils.writeDeclaredField(privatelyShadowedChild, "b", Boolean.FALSE, true);
assertEquals(Boolean.FALSE, FieldUtils.readDeclaredField(privatelyShadowedChild, "b", true));
- FieldUtils.writeDeclaredField(privatelyShadowedChild, "i", Integer.valueOf(0), true);
- assertEquals(Integer.valueOf(0), FieldUtils.readDeclaredField(privatelyShadowedChild, "i", true));
- FieldUtils.writeDeclaredField(privatelyShadowedChild, "d", Double.valueOf(0.0), true);
- assertEquals(Double.valueOf(0.0), FieldUtils.readDeclaredField(privatelyShadowedChild, "d", true));
+ FieldUtils.writeDeclaredField(privatelyShadowedChild, "i", 0, true);
+ assertEquals(0, FieldUtils.readDeclaredField(privatelyShadowedChild, "i", true));
+ FieldUtils.writeDeclaredField(privatelyShadowedChild, "d", 0.0, true);
+ assertEquals(0.0, FieldUtils.readDeclaredField(privatelyShadowedChild, "d", true));
}
@Test(expected = IllegalArgumentException.class)
Re: svn commit: r1544770 - /commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java
Posted by Maurizio Cucchiara <mc...@apache.org>.
Restored, thanks for having pointed out and for the explanation (which
sounds like a cost to pay, but at the same time it seems reasonable to
me)
Maurizio Cucchiara
On 24 November 2013 14:55, sebb <se...@gmail.com> wrote:
> On 24 November 2013 08:52, Maurizio Cucchiara <mc...@apache.org> wrote:
>> On 23 November 2013 16:05, sebb <se...@gmail.com> wrote:
>>>Indeed; auto-unboxing can generate an NPE under some circumstances.
>>> And I found at least one case where the auto-boxing was hiding a bug
>>> (sorry, no longer have the details).
>>
>> IIRC the vars I have unboxed were only constant values. I can't
>> imagine how they can generate a NPE.
>
> The constant values were using auto-boxing, which cannot generate an NPE.
> Only unboxing can generate NPE.
>
> However, the problem with allowing auto-boxing for constants is that
> it makes it hard to catch auto-boxing for variables in the same source
> file.
> If the warnings are enabled, then each of the constants will need an
> @SuppressWarning - which is even noisier than the original.
>
>>> Note also that removing explicit [un]boxing does not change the
>>> generated code, so I would not class it as an optimisation.
>>
>> The code is less verbose so I consider it a good optimisation in terms
>> of readability (calling every variables with just an alphabetic letter
>> doesn't change the generated code as well).
>
> I agree that constants like
>
> static final String S = "s";
>
> are useless.
>
> But that is because the constant name is useless, as it does not
> describe what the constant is used for.
>
>> ---------------------------------------------------------------------
>> 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
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1544770 - /commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java
Posted by sebb <se...@gmail.com>.
On 24 November 2013 08:52, Maurizio Cucchiara <mc...@apache.org> wrote:
> On 23 November 2013 16:05, sebb <se...@gmail.com> wrote:
>>Indeed; auto-unboxing can generate an NPE under some circumstances.
>> And I found at least one case where the auto-boxing was hiding a bug
>> (sorry, no longer have the details).
>
> IIRC the vars I have unboxed were only constant values. I can't
> imagine how they can generate a NPE.
The constant values were using auto-boxing, which cannot generate an NPE.
Only unboxing can generate NPE.
However, the problem with allowing auto-boxing for constants is that
it makes it hard to catch auto-boxing for variables in the same source
file.
If the warnings are enabled, then each of the constants will need an
@SuppressWarning - which is even noisier than the original.
>> Note also that removing explicit [un]boxing does not change the
>> generated code, so I would not class it as an optimisation.
>
> The code is less verbose so I consider it a good optimisation in terms
> of readability (calling every variables with just an alphabetic letter
> doesn't change the generated code as well).
I agree that constants like
static final String S = "s";
are useless.
But that is because the constant name is useless, as it does not
describe what the constant is used for.
> ---------------------------------------------------------------------
> 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: r1544770 - /commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java
Posted by Maurizio Cucchiara <mc...@apache.org>.
On 23 November 2013 16:05, sebb <se...@gmail.com> wrote:
>Indeed; auto-unboxing can generate an NPE under some circumstances.
> And I found at least one case where the auto-boxing was hiding a bug
> (sorry, no longer have the details).
IIRC the vars I have unboxed were only constant values. I can't
imagine how they can generate a NPE.
> Note also that removing explicit [un]boxing does not change the
> generated code, so I would not class it as an optimisation.
The code is less verbose so I consider it a good optimisation in terms
of readability (calling every variables with just an alphabetic letter
doesn't change the generated code as well).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1544770 - /commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java
Posted by sebb <se...@gmail.com>.
On 23 November 2013 15:33, Gary Gregory <ga...@gmail.com> wrote:
> On Sat, Nov 23, 2013 at 10:30 AM, Matt Benson <gu...@gmail.com> wrote:
>
>> Maurizio:
>> ISTR that, at the example of Sebb, we tend to avoid implicit boxing and
>> unboxing in the various Commons codebases including that of [lang].
>>
>
> That's the style I've been using in Commons and at work.
>
> The advantage of explicit boxing is that you can turn on auto-boxing
> warnings in your IDE and catch cases where you really do not want
> auto-boxing.
Indeed; auto-unboxing can generate an NPE under some circumstances.
And I found at least one case where the auto-boxing was hiding a bug
(sorry, no longer have the details).
Note also that removing explicit [un]boxing does not change the
generated code, so I would not class it as an optimisation.
> Gary
>
>
>> Matt
>> On Nov 23, 2013 5:12 AM, <mc...@apache.org> wrote:
>>
>> > Author: mcucchiara
>> > Date: Sat Nov 23 11:11:28 2013
>> > New Revision: 1544770
>> >
>> > URL: http://svn.apache.org/r1544770
>> > Log:
>> > Minor code optimizations:
>> > * removed boxing
>> > * removed unused variables
>> > * removed useless casts* removed boxing
>> > * removed unused variables
>> > * removed useless casts
>> > * replaced asArrayList method with Arrays.asList
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1544770 - /commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java
Posted by Gary Gregory <ga...@gmail.com>.
On Sat, Nov 23, 2013 at 10:30 AM, Matt Benson <gu...@gmail.com> wrote:
> Maurizio:
> ISTR that, at the example of Sebb, we tend to avoid implicit boxing and
> unboxing in the various Commons codebases including that of [lang].
>
That's the style I've been using in Commons and at work.
The advantage of explicit boxing is that you can turn on auto-boxing
warnings in your IDE and catch cases where you really do not want
auto-boxing.
Gary
> Matt
> On Nov 23, 2013 5:12 AM, <mc...@apache.org> wrote:
>
> > Author: mcucchiara
> > Date: Sat Nov 23 11:11:28 2013
> > New Revision: 1544770
> >
> > URL: http://svn.apache.org/r1544770
> > Log:
> > Minor code optimizations:
> > * removed boxing
> > * removed unused variables
> > * removed useless casts* removed boxing
> > * removed unused variables
> > * removed useless casts
> > * replaced asArrayList method with Arrays.asList
> >
> > Modified:
> >
> >
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java
> >
> > Modified:
> >
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java
> > URL:
> >
> http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java?rev=1544770&r1=1544769&r2=1544770&view=diff
> >
> >
> ==============================================================================
> > ---
> >
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java
> > (original)
> > +++
> >
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java
> > Sat Nov 23 11:11:28 2013
> > @@ -16,31 +16,20 @@
> > */
> > package org.apache.commons.lang3.reflect;
> >
> > -import static org.junit.Assert.assertArrayEquals;
> > -import static org.junit.Assert.assertEquals;
> > -import static org.junit.Assert.assertFalse;
> > -import static org.junit.Assert.assertNotNull;
> > -import static org.junit.Assert.assertNull;
> > -import static org.junit.Assert.assertTrue;
> > -import static org.junit.Assert.fail;
> > -import static org.junit.Assume.assumeNotNull;
> > +import org.apache.commons.lang3.ArrayUtils;
> > +import org.apache.commons.lang3.reflect.testbed.*;
> > +import org.junit.Before;
> > +import org.junit.Test;
> >
> > import java.lang.reflect.Constructor;
> > import java.lang.reflect.Field;
> > import java.lang.reflect.Modifier;
> > import java.util.ArrayList;
> > +import java.util.Arrays;
> > import java.util.List;
> >
> > -import org.apache.commons.lang3.ArrayUtils;
> > -import org.apache.commons.lang3.reflect.testbed.Ambig;
> > -import org.apache.commons.lang3.reflect.testbed.Foo;
> > -import org.apache.commons.lang3.reflect.testbed.PrivatelyShadowedChild;
> > -import org.apache.commons.lang3.reflect.testbed.PublicChild;
> > -import org.apache.commons.lang3.reflect.testbed.PubliclyShadowedChild;
> > -import org.apache.commons.lang3.reflect.testbed.StaticContainer;
> > -import org.apache.commons.lang3.reflect.testbed.StaticContainerChild;
> > -import org.junit.Before;
> > -import org.junit.Test;
> > +import static org.junit.Assert.*;
> > +import static org.junit.Assume.assumeNotNull;
> >
> > /**
> > * Unit tests FieldUtils
> > @@ -49,12 +38,10 @@ import org.junit.Test;
> > */
> > public class FieldUtilsTest {
> >
> > - static final String S = "s";
> > - static final String SS = "ss";
> > - static final Integer I0 = Integer.valueOf(0);
> > - static final Integer I1 = Integer.valueOf(1);
> > - static final Double D0 = Double.valueOf(0.0);
> > - static final Double D1 = Double.valueOf(1.0);
> > + static final Integer I0 = 0;
> > + static final Integer I1 = 1;
> > + static final Double D0 = 0.0;
> > + static final Double D1 = 1.0;
> >
> > private PublicChild publicChild;
> > private PubliclyShadowedChild publiclyShadowedChild;
> > @@ -167,20 +154,12 @@ public class FieldUtilsTest {
> > assertEquals(5,
> > FieldUtils.getAllFields(PublicChild.class).length);
> > }
> >
> > - private <T> List<T> asArrayList(T... values) {
> > - final ArrayList<T> arrayList = new ArrayList<T>();
> > - for (T t : values) {
> > - arrayList.add(t);
> > - }
> > - return arrayList;
> > - }
> > -
> > @Test
> > public void testGetAllFieldsList() {
> > assertEquals(0,
> FieldUtils.getAllFieldsList(Object.class).size());
> > - final List<Field> fieldsNumber =
> > asArrayList(Number.class.getDeclaredFields());
> > + final List<Field> fieldsNumber =
> > Arrays.asList(Number.class.getDeclaredFields());
> > assertEquals(fieldsNumber,
> > FieldUtils.getAllFieldsList(Number.class));
> > - final List<Field> fieldsInteger =
> > asArrayList(Integer.class.getDeclaredFields());
> > + final List<Field> fieldsInteger =
> > Arrays.asList(Integer.class.getDeclaredFields());
> > final List<Field> allFieldsInteger = new
> > ArrayList<Field>(fieldsInteger);
> > allFieldsInteger.addAll(fieldsNumber);
> > assertEquals(allFieldsInteger,
> > FieldUtils.getAllFieldsList(Integer.class));
> > @@ -462,7 +441,7 @@ public class FieldUtilsTest {
> > assertEquals(D0, FieldUtils.readField(parentD,
> > privatelyShadowedChild));
> >
> > try {
> > - FieldUtils.readField((Field) null, publicChild);
> > + FieldUtils.readField(null, publicChild);
> > fail("a null field should cause an
> IllegalArgumentException");
> > } catch (final IllegalArgumentException e) {
> > // expected
> > @@ -493,7 +472,7 @@ public class FieldUtilsTest {
> > assertEquals(D0, FieldUtils.readField(parentD,
> > privatelyShadowedChild, true));
> >
> > try {
> > - FieldUtils.readField((Field) null, publicChild, true);
> > + FieldUtils.readField(null, publicChild, true);
> > fail("a null field should cause an
> IllegalArgumentException");
> > } catch (final IllegalArgumentException e) {
> > // expected
> > @@ -643,7 +622,7 @@ public class FieldUtilsTest {
> > }
> >
> > try {
> > - FieldUtils.readDeclaredField((Object) null, "none");
> > + FieldUtils.readDeclaredField(null, "none");
> > fail("a null target should cause an
> > IllegalArgumentException");
> > } catch (final IllegalArgumentException e) {
> > // expected
> > @@ -727,7 +706,7 @@ public class FieldUtilsTest {
> > }
> >
> > try {
> > - FieldUtils.readDeclaredField((Object) null, "none", true);
> > + FieldUtils.readDeclaredField(null, "none", true);
> > fail("a null target should cause an
> > IllegalArgumentException");
> > } catch (final IllegalArgumentException e) {
> > // expected
> > @@ -839,28 +818,28 @@ public class FieldUtilsTest {
> > assertEquals("new", StaticContainer.getMutablePrivate());
> > field =
> > StaticContainer.class.getDeclaredField("IMMUTABLE_PUBLIC");
> > try {
> > - FieldUtils.writeStaticField(field, "new", true);
> > + FieldUtils.writeStaticField(field, "new", true);
> > fail("Expected IllegalAccessException");
> > } catch (final IllegalAccessException e) {
> > // pass
> > }
> > field =
> > StaticContainer.class.getDeclaredField("IMMUTABLE_PROTECTED");
> > try {
> > - FieldUtils.writeStaticField(field, "new", true);
> > + FieldUtils.writeStaticField(field, "new", true);
> > fail("Expected IllegalAccessException");
> > } catch (final IllegalAccessException e) {
> > // pass
> > }
> > field =
> > StaticContainer.class.getDeclaredField("IMMUTABLE_PACKAGE");
> > try {
> > - FieldUtils.writeStaticField(field, "new", true);
> > + FieldUtils.writeStaticField(field, "new", true);
> > fail("Expected IllegalAccessException");
> > } catch (final IllegalAccessException e) {
> > // pass
> > }
> > field =
> > StaticContainer.class.getDeclaredField("IMMUTABLE_PRIVATE");
> > try {
> > - FieldUtils.writeStaticField(field, "new", true);
> > + FieldUtils.writeStaticField(field, "new", true);
> > fail("Expected IllegalAccessException");
> > } catch (final IllegalAccessException e) {
> > // pass
> > @@ -1049,13 +1028,13 @@ public class FieldUtilsTest {
> > }
> > field = parentClass.getDeclaredField("i");
> > try {
> > - FieldUtils.writeField(field, publicChild,
> > Integer.valueOf(Integer.MAX_VALUE));
> > + FieldUtils.writeField(field, publicChild,
> Integer.MAX_VALUE);
> > } catch (final IllegalAccessException e) {
> > // pass
> > }
> > field = parentClass.getDeclaredField("d");
> > try {
> > - FieldUtils.writeField(field, publicChild,
> > Double.valueOf(Double.MAX_VALUE));
> > + FieldUtils.writeField(field, publicChild, Double.MAX_VALUE);
> > } catch (final IllegalAccessException e) {
> > // pass
> > }
> > @@ -1070,11 +1049,11 @@ public class FieldUtilsTest {
> > FieldUtils.writeField(field, publicChild, Boolean.TRUE, true);
> > assertEquals(Boolean.TRUE, field.get(publicChild));
> > field = parentClass.getDeclaredField("i");
> > - FieldUtils.writeField(field, publicChild,
> > Integer.valueOf(Integer.MAX_VALUE), true);
> > - assertEquals(Integer.valueOf(Integer.MAX_VALUE),
> > field.get(publicChild));
> > + FieldUtils.writeField(field, publicChild, Integer.MAX_VALUE,
> > true);
> > + assertEquals(Integer.MAX_VALUE, field.get(publicChild));
> > field = parentClass.getDeclaredField("d");
> > - FieldUtils.writeField(field, publicChild,
> > Double.valueOf(Double.MAX_VALUE), true);
> > - assertEquals(Double.valueOf(Double.MAX_VALUE),
> > field.get(publicChild));
> > + FieldUtils.writeField(field, publicChild, Double.MAX_VALUE,
> true);
> > + assertEquals(Double.MAX_VALUE, field.get(publicChild));
> > }
> >
> > @Test
> > @@ -1088,13 +1067,13 @@ public class FieldUtilsTest {
> > // pass
> > }
> > try {
> > - FieldUtils.writeField(publicChild, "i", Integer.valueOf(1));
> > + FieldUtils.writeField(publicChild, "i", 1);
> > fail("Expected IllegalArgumentException");
> > } catch (final IllegalArgumentException e) {
> > // pass
> > }
> > try {
> > - FieldUtils.writeField(publicChild, "d",
> Double.valueOf(1.0));
> > + FieldUtils.writeField(publicChild, "d", 1.0);
> > fail("Expected IllegalArgumentException");
> > } catch (final IllegalArgumentException e) {
> > // pass
> > @@ -1104,10 +1083,10 @@ public class FieldUtilsTest {
> > assertEquals("S", FieldUtils.readField(publiclyShadowedChild,
> > "s"));
> > FieldUtils.writeField(publiclyShadowedChild, "b",
> Boolean.FALSE);
> > assertEquals(Boolean.FALSE,
> > FieldUtils.readField(publiclyShadowedChild, "b"));
> > - FieldUtils.writeField(publiclyShadowedChild, "i",
> > Integer.valueOf(0));
> > - assertEquals(Integer.valueOf(0),
> > FieldUtils.readField(publiclyShadowedChild, "i"));
> > - FieldUtils.writeField(publiclyShadowedChild, "d",
> > Double.valueOf(0.0));
> > - assertEquals(Double.valueOf(0.0),
> > FieldUtils.readField(publiclyShadowedChild, "d"));
> > + FieldUtils.writeField(publiclyShadowedChild, "i", 0);
> > + assertEquals(0, FieldUtils.readField(publiclyShadowedChild,
> "i"));
> > + FieldUtils.writeField(publiclyShadowedChild, "d", 0.0);
> > + assertEquals(0.0, FieldUtils.readField(publiclyShadowedChild,
> > "d"));
> >
> > FieldUtils.writeField(privatelyShadowedChild, "s", "S");
> > assertEquals("S", FieldUtils.readField(privatelyShadowedChild,
> > "s"));
> > @@ -1118,13 +1097,13 @@ public class FieldUtilsTest {
> > // pass
> > }
> > try {
> > - FieldUtils.writeField(privatelyShadowedChild, "i",
> > Integer.valueOf(1));
> > + FieldUtils.writeField(privatelyShadowedChild, "i", 1);
> > fail("Expected IllegalArgumentException");
> > } catch (final IllegalArgumentException e) {
> > // pass
> > }
> > try {
> > - FieldUtils.writeField(privatelyShadowedChild, "d",
> > Double.valueOf(1.0));
> > + FieldUtils.writeField(privatelyShadowedChild, "d", 1.0);
> > fail("Expected IllegalArgumentException");
> > } catch (final IllegalArgumentException e) {
> > // pass
> > @@ -1137,28 +1116,28 @@ public class FieldUtilsTest {
> > assertEquals("S", FieldUtils.readField(publicChild, "s", true));
> > FieldUtils.writeField(publicChild, "b", Boolean.TRUE, true);
> > assertEquals(Boolean.TRUE, FieldUtils.readField(publicChild,
> "b",
> > true));
> > - FieldUtils.writeField(publicChild, "i", Integer.valueOf(1),
> true);
> > - assertEquals(Integer.valueOf(1),
> > FieldUtils.readField(publicChild, "i", true));
> > - FieldUtils.writeField(publicChild, "d", Double.valueOf(1.0),
> > true);
> > - assertEquals(Double.valueOf(1.0),
> > FieldUtils.readField(publicChild, "d", true));
> > + FieldUtils.writeField(publicChild, "i", 1, true);
> > + assertEquals(1, FieldUtils.readField(publicChild, "i", true));
> > + FieldUtils.writeField(publicChild, "d", 1.0, true);
> > + assertEquals(1.0, FieldUtils.readField(publicChild, "d", true));
> >
> > FieldUtils.writeField(publiclyShadowedChild, "s", "S", true);
> > assertEquals("S", FieldUtils.readField(publiclyShadowedChild,
> > "s", true));
> > FieldUtils.writeField(publiclyShadowedChild, "b", Boolean.FALSE,
> > true);
> > assertEquals(Boolean.FALSE,
> > FieldUtils.readField(publiclyShadowedChild, "b", true));
> > - FieldUtils.writeField(publiclyShadowedChild, "i",
> > Integer.valueOf(0), true);
> > - assertEquals(Integer.valueOf(0),
> > FieldUtils.readField(publiclyShadowedChild, "i", true));
> > - FieldUtils.writeField(publiclyShadowedChild, "d",
> > Double.valueOf(0.0), true);
> > - assertEquals(Double.valueOf(0.0),
> > FieldUtils.readField(publiclyShadowedChild, "d", true));
> > + FieldUtils.writeField(publiclyShadowedChild, "i", 0, true);
> > + assertEquals(0, FieldUtils.readField(publiclyShadowedChild, "i",
> > true));
> > + FieldUtils.writeField(publiclyShadowedChild, "d", 0.0, true);
> > + assertEquals(0.0, FieldUtils.readField(publiclyShadowedChild,
> > "d", true));
> >
> > FieldUtils.writeField(privatelyShadowedChild, "s", "S", true);
> > assertEquals("S", FieldUtils.readField(privatelyShadowedChild,
> > "s", true));
> > FieldUtils.writeField(privatelyShadowedChild, "b",
> Boolean.FALSE,
> > true);
> > assertEquals(Boolean.FALSE,
> > FieldUtils.readField(privatelyShadowedChild, "b", true));
> > - FieldUtils.writeField(privatelyShadowedChild, "i",
> > Integer.valueOf(0), true);
> > - assertEquals(Integer.valueOf(0),
> > FieldUtils.readField(privatelyShadowedChild, "i", true));
> > - FieldUtils.writeField(privatelyShadowedChild, "d",
> > Double.valueOf(0.0), true);
> > - assertEquals(Double.valueOf(0.0),
> > FieldUtils.readField(privatelyShadowedChild, "d", true));
> > + FieldUtils.writeField(privatelyShadowedChild, "i", 0, true);
> > + assertEquals(0, FieldUtils.readField(privatelyShadowedChild,
> "i",
> > true));
> > + FieldUtils.writeField(privatelyShadowedChild, "d", 0.0, true);
> > + assertEquals(0.0, FieldUtils.readField(privatelyShadowedChild,
> > "d", true));
> > }
> >
> > @Test
> > @@ -1176,13 +1155,13 @@ public class FieldUtilsTest {
> > // pass
> > }
> > try {
> > - FieldUtils.writeDeclaredField(publicChild, "i",
> > Integer.valueOf(1));
> > + FieldUtils.writeDeclaredField(publicChild, "i", 1);
> > fail("Expected IllegalArgumentException");
> > } catch (final IllegalArgumentException e) {
> > // pass
> > }
> > try {
> > - FieldUtils.writeDeclaredField(publicChild, "d",
> > Double.valueOf(1.0));
> > + FieldUtils.writeDeclaredField(publicChild, "d", 1.0);
> > fail("Expected IllegalArgumentException");
> > } catch (final IllegalArgumentException e) {
> > // pass
> > @@ -1192,10 +1171,10 @@ public class FieldUtilsTest {
> > assertEquals("S",
> > FieldUtils.readDeclaredField(publiclyShadowedChild, "s"));
> > FieldUtils.writeDeclaredField(publiclyShadowedChild, "b",
> > Boolean.FALSE);
> > assertEquals(Boolean.FALSE,
> > FieldUtils.readDeclaredField(publiclyShadowedChild, "b"));
> > - FieldUtils.writeDeclaredField(publiclyShadowedChild, "i",
> > Integer.valueOf(0));
> > - assertEquals(Integer.valueOf(0),
> > FieldUtils.readDeclaredField(publiclyShadowedChild, "i"));
> > - FieldUtils.writeDeclaredField(publiclyShadowedChild, "d",
> > Double.valueOf(0.0));
> > - assertEquals(Double.valueOf(0.0),
> > FieldUtils.readDeclaredField(publiclyShadowedChild, "d"));
> > + FieldUtils.writeDeclaredField(publiclyShadowedChild, "i", 0);
> > + assertEquals(0,
> > FieldUtils.readDeclaredField(publiclyShadowedChild, "i"));
> > + FieldUtils.writeDeclaredField(publiclyShadowedChild, "d", 0.0);
> > + assertEquals(0.0,
> > FieldUtils.readDeclaredField(publiclyShadowedChild, "d"));
> >
> > try {
> > FieldUtils.writeDeclaredField(privatelyShadowedChild, "s",
> > "S");
> > @@ -1210,13 +1189,13 @@ public class FieldUtilsTest {
> > // pass
> > }
> > try {
> > - FieldUtils.writeDeclaredField(privatelyShadowedChild, "i",
> > Integer.valueOf(1));
> > + FieldUtils.writeDeclaredField(privatelyShadowedChild, "i",
> 1);
> > fail("Expected IllegalArgumentException");
> > } catch (final IllegalArgumentException e) {
> > // pass
> > }
> > try {
> > - FieldUtils.writeDeclaredField(privatelyShadowedChild, "d",
> > Double.valueOf(1.0));
> > + FieldUtils.writeDeclaredField(privatelyShadowedChild, "d",
> > 1.0);
> > fail("Expected IllegalArgumentException");
> > } catch (final IllegalArgumentException e) {
> > // pass
> > @@ -1238,13 +1217,13 @@ public class FieldUtilsTest {
> > // pass
> > }
> > try {
> > - FieldUtils.writeDeclaredField(publicChild, "i",
> > Integer.valueOf(1), true);
> > + FieldUtils.writeDeclaredField(publicChild, "i", 1, true);
> > fail("Expected IllegalArgumentException");
> > } catch (final IllegalArgumentException e) {
> > // pass
> > }
> > try {
> > - FieldUtils.writeDeclaredField(publicChild, "d",
> > Double.valueOf(1.0), true);
> > + FieldUtils.writeDeclaredField(publicChild, "d", 1.0, true);
> > fail("Expected IllegalArgumentException");
> > } catch (final IllegalArgumentException e) {
> > // pass
> > @@ -1254,19 +1233,19 @@ public class FieldUtilsTest {
> > assertEquals("S",
> > FieldUtils.readDeclaredField(publiclyShadowedChild, "s", true));
> > FieldUtils.writeDeclaredField(publiclyShadowedChild, "b",
> > Boolean.FALSE, true);
> > assertEquals(Boolean.FALSE,
> > FieldUtils.readDeclaredField(publiclyShadowedChild, "b", true));
> > - FieldUtils.writeDeclaredField(publiclyShadowedChild, "i",
> > Integer.valueOf(0), true);
> > - assertEquals(Integer.valueOf(0),
> > FieldUtils.readDeclaredField(publiclyShadowedChild, "i", true));
> > - FieldUtils.writeDeclaredField(publiclyShadowedChild, "d",
> > Double.valueOf(0.0), true);
> > - assertEquals(Double.valueOf(0.0),
> > FieldUtils.readDeclaredField(publiclyShadowedChild, "d", true));
> > + FieldUtils.writeDeclaredField(publiclyShadowedChild, "i", 0,
> > true);
> > + assertEquals(0,
> > FieldUtils.readDeclaredField(publiclyShadowedChild, "i", true));
> > + FieldUtils.writeDeclaredField(publiclyShadowedChild, "d", 0.0,
> > true);
> > + assertEquals(0.0,
> > FieldUtils.readDeclaredField(publiclyShadowedChild, "d", true));
> >
> > FieldUtils.writeDeclaredField(privatelyShadowedChild, "s", "S",
> > true);
> > assertEquals("S",
> > FieldUtils.readDeclaredField(privatelyShadowedChild, "s", true));
> > FieldUtils.writeDeclaredField(privatelyShadowedChild, "b",
> > Boolean.FALSE, true);
> > assertEquals(Boolean.FALSE,
> > FieldUtils.readDeclaredField(privatelyShadowedChild, "b", true));
> > - FieldUtils.writeDeclaredField(privatelyShadowedChild, "i",
> > Integer.valueOf(0), true);
> > - assertEquals(Integer.valueOf(0),
> > FieldUtils.readDeclaredField(privatelyShadowedChild, "i", true));
> > - FieldUtils.writeDeclaredField(privatelyShadowedChild, "d",
> > Double.valueOf(0.0), true);
> > - assertEquals(Double.valueOf(0.0),
> > FieldUtils.readDeclaredField(privatelyShadowedChild, "d", true));
> > + FieldUtils.writeDeclaredField(privatelyShadowedChild, "i", 0,
> > true);
> > + assertEquals(0,
> > FieldUtils.readDeclaredField(privatelyShadowedChild, "i", true));
> > + FieldUtils.writeDeclaredField(privatelyShadowedChild, "d", 0.0,
> > true);
> > + assertEquals(0.0,
> > FieldUtils.readDeclaredField(privatelyShadowedChild, "d", true));
> > }
> >
> > @Test(expected = IllegalArgumentException.class)
> >
> >
> >
>
--
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory
Re: svn commit: r1544770 - /commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java
Posted by Matt Benson <gu...@gmail.com>.
Maurizio:
ISTR that, at the example of Sebb, we tend to avoid implicit boxing and
unboxing in the various Commons codebases including that of [lang].
Matt
On Nov 23, 2013 5:12 AM, <mc...@apache.org> wrote:
> Author: mcucchiara
> Date: Sat Nov 23 11:11:28 2013
> New Revision: 1544770
>
> URL: http://svn.apache.org/r1544770
> Log:
> Minor code optimizations:
> * removed boxing
> * removed unused variables
> * removed useless casts* removed boxing
> * removed unused variables
> * removed useless casts
> * replaced asArrayList method with Arrays.asList
>
> Modified:
>
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java
>
> Modified:
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java
> URL:
> http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java?rev=1544770&r1=1544769&r2=1544770&view=diff
>
> ==============================================================================
> ---
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java
> (original)
> +++
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java
> Sat Nov 23 11:11:28 2013
> @@ -16,31 +16,20 @@
> */
> package org.apache.commons.lang3.reflect;
>
> -import static org.junit.Assert.assertArrayEquals;
> -import static org.junit.Assert.assertEquals;
> -import static org.junit.Assert.assertFalse;
> -import static org.junit.Assert.assertNotNull;
> -import static org.junit.Assert.assertNull;
> -import static org.junit.Assert.assertTrue;
> -import static org.junit.Assert.fail;
> -import static org.junit.Assume.assumeNotNull;
> +import org.apache.commons.lang3.ArrayUtils;
> +import org.apache.commons.lang3.reflect.testbed.*;
> +import org.junit.Before;
> +import org.junit.Test;
>
> import java.lang.reflect.Constructor;
> import java.lang.reflect.Field;
> import java.lang.reflect.Modifier;
> import java.util.ArrayList;
> +import java.util.Arrays;
> import java.util.List;
>
> -import org.apache.commons.lang3.ArrayUtils;
> -import org.apache.commons.lang3.reflect.testbed.Ambig;
> -import org.apache.commons.lang3.reflect.testbed.Foo;
> -import org.apache.commons.lang3.reflect.testbed.PrivatelyShadowedChild;
> -import org.apache.commons.lang3.reflect.testbed.PublicChild;
> -import org.apache.commons.lang3.reflect.testbed.PubliclyShadowedChild;
> -import org.apache.commons.lang3.reflect.testbed.StaticContainer;
> -import org.apache.commons.lang3.reflect.testbed.StaticContainerChild;
> -import org.junit.Before;
> -import org.junit.Test;
> +import static org.junit.Assert.*;
> +import static org.junit.Assume.assumeNotNull;
>
> /**
> * Unit tests FieldUtils
> @@ -49,12 +38,10 @@ import org.junit.Test;
> */
> public class FieldUtilsTest {
>
> - static final String S = "s";
> - static final String SS = "ss";
> - static final Integer I0 = Integer.valueOf(0);
> - static final Integer I1 = Integer.valueOf(1);
> - static final Double D0 = Double.valueOf(0.0);
> - static final Double D1 = Double.valueOf(1.0);
> + static final Integer I0 = 0;
> + static final Integer I1 = 1;
> + static final Double D0 = 0.0;
> + static final Double D1 = 1.0;
>
> private PublicChild publicChild;
> private PubliclyShadowedChild publiclyShadowedChild;
> @@ -167,20 +154,12 @@ public class FieldUtilsTest {
> assertEquals(5,
> FieldUtils.getAllFields(PublicChild.class).length);
> }
>
> - private <T> List<T> asArrayList(T... values) {
> - final ArrayList<T> arrayList = new ArrayList<T>();
> - for (T t : values) {
> - arrayList.add(t);
> - }
> - return arrayList;
> - }
> -
> @Test
> public void testGetAllFieldsList() {
> assertEquals(0, FieldUtils.getAllFieldsList(Object.class).size());
> - final List<Field> fieldsNumber =
> asArrayList(Number.class.getDeclaredFields());
> + final List<Field> fieldsNumber =
> Arrays.asList(Number.class.getDeclaredFields());
> assertEquals(fieldsNumber,
> FieldUtils.getAllFieldsList(Number.class));
> - final List<Field> fieldsInteger =
> asArrayList(Integer.class.getDeclaredFields());
> + final List<Field> fieldsInteger =
> Arrays.asList(Integer.class.getDeclaredFields());
> final List<Field> allFieldsInteger = new
> ArrayList<Field>(fieldsInteger);
> allFieldsInteger.addAll(fieldsNumber);
> assertEquals(allFieldsInteger,
> FieldUtils.getAllFieldsList(Integer.class));
> @@ -462,7 +441,7 @@ public class FieldUtilsTest {
> assertEquals(D0, FieldUtils.readField(parentD,
> privatelyShadowedChild));
>
> try {
> - FieldUtils.readField((Field) null, publicChild);
> + FieldUtils.readField(null, publicChild);
> fail("a null field should cause an IllegalArgumentException");
> } catch (final IllegalArgumentException e) {
> // expected
> @@ -493,7 +472,7 @@ public class FieldUtilsTest {
> assertEquals(D0, FieldUtils.readField(parentD,
> privatelyShadowedChild, true));
>
> try {
> - FieldUtils.readField((Field) null, publicChild, true);
> + FieldUtils.readField(null, publicChild, true);
> fail("a null field should cause an IllegalArgumentException");
> } catch (final IllegalArgumentException e) {
> // expected
> @@ -643,7 +622,7 @@ public class FieldUtilsTest {
> }
>
> try {
> - FieldUtils.readDeclaredField((Object) null, "none");
> + FieldUtils.readDeclaredField(null, "none");
> fail("a null target should cause an
> IllegalArgumentException");
> } catch (final IllegalArgumentException e) {
> // expected
> @@ -727,7 +706,7 @@ public class FieldUtilsTest {
> }
>
> try {
> - FieldUtils.readDeclaredField((Object) null, "none", true);
> + FieldUtils.readDeclaredField(null, "none", true);
> fail("a null target should cause an
> IllegalArgumentException");
> } catch (final IllegalArgumentException e) {
> // expected
> @@ -839,28 +818,28 @@ public class FieldUtilsTest {
> assertEquals("new", StaticContainer.getMutablePrivate());
> field =
> StaticContainer.class.getDeclaredField("IMMUTABLE_PUBLIC");
> try {
> - FieldUtils.writeStaticField(field, "new", true);
> + FieldUtils.writeStaticField(field, "new", true);
> fail("Expected IllegalAccessException");
> } catch (final IllegalAccessException e) {
> // pass
> }
> field =
> StaticContainer.class.getDeclaredField("IMMUTABLE_PROTECTED");
> try {
> - FieldUtils.writeStaticField(field, "new", true);
> + FieldUtils.writeStaticField(field, "new", true);
> fail("Expected IllegalAccessException");
> } catch (final IllegalAccessException e) {
> // pass
> }
> field =
> StaticContainer.class.getDeclaredField("IMMUTABLE_PACKAGE");
> try {
> - FieldUtils.writeStaticField(field, "new", true);
> + FieldUtils.writeStaticField(field, "new", true);
> fail("Expected IllegalAccessException");
> } catch (final IllegalAccessException e) {
> // pass
> }
> field =
> StaticContainer.class.getDeclaredField("IMMUTABLE_PRIVATE");
> try {
> - FieldUtils.writeStaticField(field, "new", true);
> + FieldUtils.writeStaticField(field, "new", true);
> fail("Expected IllegalAccessException");
> } catch (final IllegalAccessException e) {
> // pass
> @@ -1049,13 +1028,13 @@ public class FieldUtilsTest {
> }
> field = parentClass.getDeclaredField("i");
> try {
> - FieldUtils.writeField(field, publicChild,
> Integer.valueOf(Integer.MAX_VALUE));
> + FieldUtils.writeField(field, publicChild, Integer.MAX_VALUE);
> } catch (final IllegalAccessException e) {
> // pass
> }
> field = parentClass.getDeclaredField("d");
> try {
> - FieldUtils.writeField(field, publicChild,
> Double.valueOf(Double.MAX_VALUE));
> + FieldUtils.writeField(field, publicChild, Double.MAX_VALUE);
> } catch (final IllegalAccessException e) {
> // pass
> }
> @@ -1070,11 +1049,11 @@ public class FieldUtilsTest {
> FieldUtils.writeField(field, publicChild, Boolean.TRUE, true);
> assertEquals(Boolean.TRUE, field.get(publicChild));
> field = parentClass.getDeclaredField("i");
> - FieldUtils.writeField(field, publicChild,
> Integer.valueOf(Integer.MAX_VALUE), true);
> - assertEquals(Integer.valueOf(Integer.MAX_VALUE),
> field.get(publicChild));
> + FieldUtils.writeField(field, publicChild, Integer.MAX_VALUE,
> true);
> + assertEquals(Integer.MAX_VALUE, field.get(publicChild));
> field = parentClass.getDeclaredField("d");
> - FieldUtils.writeField(field, publicChild,
> Double.valueOf(Double.MAX_VALUE), true);
> - assertEquals(Double.valueOf(Double.MAX_VALUE),
> field.get(publicChild));
> + FieldUtils.writeField(field, publicChild, Double.MAX_VALUE, true);
> + assertEquals(Double.MAX_VALUE, field.get(publicChild));
> }
>
> @Test
> @@ -1088,13 +1067,13 @@ public class FieldUtilsTest {
> // pass
> }
> try {
> - FieldUtils.writeField(publicChild, "i", Integer.valueOf(1));
> + FieldUtils.writeField(publicChild, "i", 1);
> fail("Expected IllegalArgumentException");
> } catch (final IllegalArgumentException e) {
> // pass
> }
> try {
> - FieldUtils.writeField(publicChild, "d", Double.valueOf(1.0));
> + FieldUtils.writeField(publicChild, "d", 1.0);
> fail("Expected IllegalArgumentException");
> } catch (final IllegalArgumentException e) {
> // pass
> @@ -1104,10 +1083,10 @@ public class FieldUtilsTest {
> assertEquals("S", FieldUtils.readField(publiclyShadowedChild,
> "s"));
> FieldUtils.writeField(publiclyShadowedChild, "b", Boolean.FALSE);
> assertEquals(Boolean.FALSE,
> FieldUtils.readField(publiclyShadowedChild, "b"));
> - FieldUtils.writeField(publiclyShadowedChild, "i",
> Integer.valueOf(0));
> - assertEquals(Integer.valueOf(0),
> FieldUtils.readField(publiclyShadowedChild, "i"));
> - FieldUtils.writeField(publiclyShadowedChild, "d",
> Double.valueOf(0.0));
> - assertEquals(Double.valueOf(0.0),
> FieldUtils.readField(publiclyShadowedChild, "d"));
> + FieldUtils.writeField(publiclyShadowedChild, "i", 0);
> + assertEquals(0, FieldUtils.readField(publiclyShadowedChild, "i"));
> + FieldUtils.writeField(publiclyShadowedChild, "d", 0.0);
> + assertEquals(0.0, FieldUtils.readField(publiclyShadowedChild,
> "d"));
>
> FieldUtils.writeField(privatelyShadowedChild, "s", "S");
> assertEquals("S", FieldUtils.readField(privatelyShadowedChild,
> "s"));
> @@ -1118,13 +1097,13 @@ public class FieldUtilsTest {
> // pass
> }
> try {
> - FieldUtils.writeField(privatelyShadowedChild, "i",
> Integer.valueOf(1));
> + FieldUtils.writeField(privatelyShadowedChild, "i", 1);
> fail("Expected IllegalArgumentException");
> } catch (final IllegalArgumentException e) {
> // pass
> }
> try {
> - FieldUtils.writeField(privatelyShadowedChild, "d",
> Double.valueOf(1.0));
> + FieldUtils.writeField(privatelyShadowedChild, "d", 1.0);
> fail("Expected IllegalArgumentException");
> } catch (final IllegalArgumentException e) {
> // pass
> @@ -1137,28 +1116,28 @@ public class FieldUtilsTest {
> assertEquals("S", FieldUtils.readField(publicChild, "s", true));
> FieldUtils.writeField(publicChild, "b", Boolean.TRUE, true);
> assertEquals(Boolean.TRUE, FieldUtils.readField(publicChild, "b",
> true));
> - FieldUtils.writeField(publicChild, "i", Integer.valueOf(1), true);
> - assertEquals(Integer.valueOf(1),
> FieldUtils.readField(publicChild, "i", true));
> - FieldUtils.writeField(publicChild, "d", Double.valueOf(1.0),
> true);
> - assertEquals(Double.valueOf(1.0),
> FieldUtils.readField(publicChild, "d", true));
> + FieldUtils.writeField(publicChild, "i", 1, true);
> + assertEquals(1, FieldUtils.readField(publicChild, "i", true));
> + FieldUtils.writeField(publicChild, "d", 1.0, true);
> + assertEquals(1.0, FieldUtils.readField(publicChild, "d", true));
>
> FieldUtils.writeField(publiclyShadowedChild, "s", "S", true);
> assertEquals("S", FieldUtils.readField(publiclyShadowedChild,
> "s", true));
> FieldUtils.writeField(publiclyShadowedChild, "b", Boolean.FALSE,
> true);
> assertEquals(Boolean.FALSE,
> FieldUtils.readField(publiclyShadowedChild, "b", true));
> - FieldUtils.writeField(publiclyShadowedChild, "i",
> Integer.valueOf(0), true);
> - assertEquals(Integer.valueOf(0),
> FieldUtils.readField(publiclyShadowedChild, "i", true));
> - FieldUtils.writeField(publiclyShadowedChild, "d",
> Double.valueOf(0.0), true);
> - assertEquals(Double.valueOf(0.0),
> FieldUtils.readField(publiclyShadowedChild, "d", true));
> + FieldUtils.writeField(publiclyShadowedChild, "i", 0, true);
> + assertEquals(0, FieldUtils.readField(publiclyShadowedChild, "i",
> true));
> + FieldUtils.writeField(publiclyShadowedChild, "d", 0.0, true);
> + assertEquals(0.0, FieldUtils.readField(publiclyShadowedChild,
> "d", true));
>
> FieldUtils.writeField(privatelyShadowedChild, "s", "S", true);
> assertEquals("S", FieldUtils.readField(privatelyShadowedChild,
> "s", true));
> FieldUtils.writeField(privatelyShadowedChild, "b", Boolean.FALSE,
> true);
> assertEquals(Boolean.FALSE,
> FieldUtils.readField(privatelyShadowedChild, "b", true));
> - FieldUtils.writeField(privatelyShadowedChild, "i",
> Integer.valueOf(0), true);
> - assertEquals(Integer.valueOf(0),
> FieldUtils.readField(privatelyShadowedChild, "i", true));
> - FieldUtils.writeField(privatelyShadowedChild, "d",
> Double.valueOf(0.0), true);
> - assertEquals(Double.valueOf(0.0),
> FieldUtils.readField(privatelyShadowedChild, "d", true));
> + FieldUtils.writeField(privatelyShadowedChild, "i", 0, true);
> + assertEquals(0, FieldUtils.readField(privatelyShadowedChild, "i",
> true));
> + FieldUtils.writeField(privatelyShadowedChild, "d", 0.0, true);
> + assertEquals(0.0, FieldUtils.readField(privatelyShadowedChild,
> "d", true));
> }
>
> @Test
> @@ -1176,13 +1155,13 @@ public class FieldUtilsTest {
> // pass
> }
> try {
> - FieldUtils.writeDeclaredField(publicChild, "i",
> Integer.valueOf(1));
> + FieldUtils.writeDeclaredField(publicChild, "i", 1);
> fail("Expected IllegalArgumentException");
> } catch (final IllegalArgumentException e) {
> // pass
> }
> try {
> - FieldUtils.writeDeclaredField(publicChild, "d",
> Double.valueOf(1.0));
> + FieldUtils.writeDeclaredField(publicChild, "d", 1.0);
> fail("Expected IllegalArgumentException");
> } catch (final IllegalArgumentException e) {
> // pass
> @@ -1192,10 +1171,10 @@ public class FieldUtilsTest {
> assertEquals("S",
> FieldUtils.readDeclaredField(publiclyShadowedChild, "s"));
> FieldUtils.writeDeclaredField(publiclyShadowedChild, "b",
> Boolean.FALSE);
> assertEquals(Boolean.FALSE,
> FieldUtils.readDeclaredField(publiclyShadowedChild, "b"));
> - FieldUtils.writeDeclaredField(publiclyShadowedChild, "i",
> Integer.valueOf(0));
> - assertEquals(Integer.valueOf(0),
> FieldUtils.readDeclaredField(publiclyShadowedChild, "i"));
> - FieldUtils.writeDeclaredField(publiclyShadowedChild, "d",
> Double.valueOf(0.0));
> - assertEquals(Double.valueOf(0.0),
> FieldUtils.readDeclaredField(publiclyShadowedChild, "d"));
> + FieldUtils.writeDeclaredField(publiclyShadowedChild, "i", 0);
> + assertEquals(0,
> FieldUtils.readDeclaredField(publiclyShadowedChild, "i"));
> + FieldUtils.writeDeclaredField(publiclyShadowedChild, "d", 0.0);
> + assertEquals(0.0,
> FieldUtils.readDeclaredField(publiclyShadowedChild, "d"));
>
> try {
> FieldUtils.writeDeclaredField(privatelyShadowedChild, "s",
> "S");
> @@ -1210,13 +1189,13 @@ public class FieldUtilsTest {
> // pass
> }
> try {
> - FieldUtils.writeDeclaredField(privatelyShadowedChild, "i",
> Integer.valueOf(1));
> + FieldUtils.writeDeclaredField(privatelyShadowedChild, "i", 1);
> fail("Expected IllegalArgumentException");
> } catch (final IllegalArgumentException e) {
> // pass
> }
> try {
> - FieldUtils.writeDeclaredField(privatelyShadowedChild, "d",
> Double.valueOf(1.0));
> + FieldUtils.writeDeclaredField(privatelyShadowedChild, "d",
> 1.0);
> fail("Expected IllegalArgumentException");
> } catch (final IllegalArgumentException e) {
> // pass
> @@ -1238,13 +1217,13 @@ public class FieldUtilsTest {
> // pass
> }
> try {
> - FieldUtils.writeDeclaredField(publicChild, "i",
> Integer.valueOf(1), true);
> + FieldUtils.writeDeclaredField(publicChild, "i", 1, true);
> fail("Expected IllegalArgumentException");
> } catch (final IllegalArgumentException e) {
> // pass
> }
> try {
> - FieldUtils.writeDeclaredField(publicChild, "d",
> Double.valueOf(1.0), true);
> + FieldUtils.writeDeclaredField(publicChild, "d", 1.0, true);
> fail("Expected IllegalArgumentException");
> } catch (final IllegalArgumentException e) {
> // pass
> @@ -1254,19 +1233,19 @@ public class FieldUtilsTest {
> assertEquals("S",
> FieldUtils.readDeclaredField(publiclyShadowedChild, "s", true));
> FieldUtils.writeDeclaredField(publiclyShadowedChild, "b",
> Boolean.FALSE, true);
> assertEquals(Boolean.FALSE,
> FieldUtils.readDeclaredField(publiclyShadowedChild, "b", true));
> - FieldUtils.writeDeclaredField(publiclyShadowedChild, "i",
> Integer.valueOf(0), true);
> - assertEquals(Integer.valueOf(0),
> FieldUtils.readDeclaredField(publiclyShadowedChild, "i", true));
> - FieldUtils.writeDeclaredField(publiclyShadowedChild, "d",
> Double.valueOf(0.0), true);
> - assertEquals(Double.valueOf(0.0),
> FieldUtils.readDeclaredField(publiclyShadowedChild, "d", true));
> + FieldUtils.writeDeclaredField(publiclyShadowedChild, "i", 0,
> true);
> + assertEquals(0,
> FieldUtils.readDeclaredField(publiclyShadowedChild, "i", true));
> + FieldUtils.writeDeclaredField(publiclyShadowedChild, "d", 0.0,
> true);
> + assertEquals(0.0,
> FieldUtils.readDeclaredField(publiclyShadowedChild, "d", true));
>
> FieldUtils.writeDeclaredField(privatelyShadowedChild, "s", "S",
> true);
> assertEquals("S",
> FieldUtils.readDeclaredField(privatelyShadowedChild, "s", true));
> FieldUtils.writeDeclaredField(privatelyShadowedChild, "b",
> Boolean.FALSE, true);
> assertEquals(Boolean.FALSE,
> FieldUtils.readDeclaredField(privatelyShadowedChild, "b", true));
> - FieldUtils.writeDeclaredField(privatelyShadowedChild, "i",
> Integer.valueOf(0), true);
> - assertEquals(Integer.valueOf(0),
> FieldUtils.readDeclaredField(privatelyShadowedChild, "i", true));
> - FieldUtils.writeDeclaredField(privatelyShadowedChild, "d",
> Double.valueOf(0.0), true);
> - assertEquals(Double.valueOf(0.0),
> FieldUtils.readDeclaredField(privatelyShadowedChild, "d", true));
> + FieldUtils.writeDeclaredField(privatelyShadowedChild, "i", 0,
> true);
> + assertEquals(0,
> FieldUtils.readDeclaredField(privatelyShadowedChild, "i", true));
> + FieldUtils.writeDeclaredField(privatelyShadowedChild, "d", 0.0,
> true);
> + assertEquals(0.0,
> FieldUtils.readDeclaredField(privatelyShadowedChild, "d", true));
> }
>
> @Test(expected = IllegalArgumentException.class)
>
>
>