You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/12/31 17:30:12 UTC

[GitHub] [commons-beanutils] garydgregory commented on a change in pull request #49: Updated Existing Converters

garydgregory commented on a change in pull request #49:
URL: https://github.com/apache/commons-beanutils/pull/49#discussion_r550548711



##########
File path: src/main/java/org/apache/commons/beanutils2/converters/CharacterConverter.java
##########
@@ -83,7 +94,19 @@ protected String convertToString(final Object value) {
     @Override
     protected <T> T convertToType(final Class<T> type, final Object value) throws Exception {
         if (Character.class.equals(type) || Character.TYPE.equals(type)) {
-            return type.cast(Character.valueOf(value.toString().charAt(0)));
+            final String stringValue = toString(value);
+
+            if (stringValue.isEmpty()) {
+                throw new IllegalArgumentException("Value can't be empty.");
+            }
+
+            if (stringValue.toLowerCase().startsWith(HEX_PREFIX)) {
+                final String substring = stringValue.substring(HEX_PREFIX.length());
+                final int hex = Integer.parseInt(substring, 16);
+                return type.cast((char)hex);

Review comment:
       Missing space after type cast.

##########
File path: src/main/java/org/apache/commons/beanutils2/converters/CharacterConverter.java
##########
@@ -83,7 +94,19 @@ protected String convertToString(final Object value) {
     @Override
     protected <T> T convertToType(final Class<T> type, final Object value) throws Exception {
         if (Character.class.equals(type) || Character.TYPE.equals(type)) {
-            return type.cast(Character.valueOf(value.toString().charAt(0)));
+            final String stringValue = toString(value);
+
+            if (stringValue.isEmpty()) {
+                throw new IllegalArgumentException("Value can't be empty.");
+            }
+
+            if (stringValue.toLowerCase().startsWith(HEX_PREFIX)) {

Review comment:
       Hi @SethFalco,
   
   I don't think you want to convert the _whole_ string just to test the 1st 2 chars.

##########
File path: src/main/java/org/apache/commons/beanutils2/converters/EnumConverter.java
##########
@@ -71,21 +79,42 @@ public EnumConverter(final Object defaultValue) {
      */
     @SuppressWarnings({ "rawtypes" })
     @Override
-    protected <T> T  convertToType(final Class<T> type, final Object value) throws Throwable {
+    protected <T> T convertToType(final Class<T> type, final Object value) throws Throwable {
         if (Enum.class.isAssignableFrom(type)) {
-            final String enumValue = String.valueOf(value);
-            final T[] constants = type.getEnumConstants();
-            if (constants == null) {
-                throw conversionException(type, value);
+            final String stringValue = toString(value);
+
+            try {
+                return type.cast((Enum)Enum.valueOf((Class)type, stringValue));

Review comment:
       Space after type cast.
   

##########
File path: src/main/java/org/apache/commons/beanutils2/converters/InstantConverter.java
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.beanutils2.converters;
+
+import java.time.Instant;
+
+/**
+ * {@link DateTimeConverter} implementation that handles conversion to
+ * and from <b>java.time.LocalDateTime</b> objects.
+ * <p>
+ * This implementation can be configured to handle conversion either
+ * by using a Locale's default format or by specifying a set of format
+ * patterns (note, there is no default String conversion for Calendar).
+ * See the {@link DateTimeConverter} documentation for further details.
+ * </p>
+ * <p>
+ * Can be configured to either return a <i>default value</i> or throw a
+ * {@code ConversionException} if a conversion error occurs.
+ * </p>
+ *
+ * @since 2.0
+ * @see Instant
+ */
+public final class InstantConverter extends DateTimeConverter {
+
+    /**
+     * Construct a <b>java.time.LocalDateTime</b> <i>Converter</i> that throws
+     * a {@code ConversionException} if an error occurs.
+     */
+    public InstantConverter() {
+        super();
+    }
+
+    /**
+     * Construct a <b>java.time.LocalDateTime</b> <i>Converter</i> that returns
+     * a default value if an error occurs.
+     *
+     * @param defaultValue The default value to be returned
+     * if the value to be converted is missing or an error
+     * occurs converting the value.
+     */
+    public InstantConverter(final Object defaultValue) {
+        super(defaultValue);
+    }
+
+    /**
+     * Return the default type this {@code Converter} handles.

Review comment:
       "Return" -> "Gets"
   

##########
File path: src/main/java/org/apache/commons/beanutils2/converters/PeriodConverter.java
##########
@@ -74,7 +87,19 @@ public PeriodConverter(final Object defaultValue) {
     @Override
     protected <T> T convertToType(final Class<T> type, final Object value) throws Throwable {
         if (Period.class.equals(type)) {
-            return type.cast(Period.parse((String.valueOf(value))));
+            final String stringValue = toString(value);
+
+            try {
+                return type.cast(Period.parse(stringValue));
+            } catch (DateTimeParseException ex) {
+                try {
+                    int number = Integer.parseInt(stringValue);

Review comment:
       Upon revisitation **-1**: Same comment as for Duration.

##########
File path: src/test/java/org/apache/commons/beanutils2/converters/ConverterTestSuite.java
##########
@@ -41,7 +41,6 @@ public static Test suite() {
         testSuite.addTestSuite(BooleanConverterTestCase.class);
         testSuite.addTestSuite(ByteConverterTestCase.class);
         testSuite.addTestSuite(CalendarConverterTestCase.class);
-        testSuite.addTestSuite(CharacterConverterTestCase.class);
         testSuite.addTestSuite(ClassConverterTestCase.class);

Review comment:
       Not sure why you deleted this class from this list. What am I missing? Please explain. The class is gone from git master anyway but I am still curious. Please rebase :-)

##########
File path: src/main/java/org/apache/commons/beanutils2/converters/DurationConverter.java
##########
@@ -74,10 +84,21 @@ public DurationConverter(final Object defaultValue) {
     @Override
     protected <T> T convertToType(final Class<T> type, final Object value) throws Throwable {
         if (Duration.class.equals(type)) {
-            return type.cast(Duration.parse((String.valueOf(value))));
+            final String stringValue = toString(value);
+
+            try {
+                return type.cast(Duration.parse(stringValue));
+            } catch (DateTimeParseException ex) {
+                try {
+                    long number = Long.parseLong(stringValue);
+                    return type.cast(Duration.of(number, ChronoUnit.MILLIS));

Review comment:
       **-1**: This feels like a recipe for problems and confusion, the code expects _some_ number in _some_ format in _some_ scale. This is just not a Duration which is a well defined concept. 
   

##########
File path: src/main/java/org/apache/commons/beanutils2/converters/DateTimeConverter.java
##########
@@ -360,6 +364,11 @@ protected String convertToString(final Object value) throws Throwable {
             return toDate(targetType, date.toInstant().toEpochMilli());
         }
 
+        if (value instanceof Instant) {
+            final Instant date = (Instant)value;

Review comment:
       Missing space after type cast.
   

##########
File path: src/main/java/org/apache/commons/beanutils2/converters/DateTimeConverter.java
##########
@@ -530,6 +545,14 @@ protected String convertToString(final Object value) throws Throwable {
             }
         }
 
+        if (type.equals(Instant.class)) {
+            try {
+                return type.cast(Instant.parse(value));
+            } catch (final RuntimeException ex) {
+                throw new ConversionException("String must be in ISO-8601 format to create a java.time.Instant");

Review comment:
       That does not seem right because there are a lot of exceptions that will not match the new message, the catch clause should be more specific IMO.

##########
File path: src/main/java/org/apache/commons/beanutils2/converters/DateTimeConverter.java
##########
@@ -454,6 +463,11 @@ protected String convertToString(final Object value) throws Throwable {
             return type.cast(offsetDateTime);
         }
 
+        if (type.equals(Instant.class)) {
+            final Instant instant = Instant.ofEpochMilli(value);
+            return type.cast(instant);

Review comment:
       No need for an `instant` local var IMO, you can inline it.
   

##########
File path: src/main/java/org/apache/commons/beanutils2/converters/EnumConverter.java
##########
@@ -71,21 +79,42 @@ public EnumConverter(final Object defaultValue) {
      */
     @SuppressWarnings({ "rawtypes" })
     @Override
-    protected <T> T  convertToType(final Class<T> type, final Object value) throws Throwable {
+    protected <T> T convertToType(final Class<T> type, final Object value) throws Throwable {
         if (Enum.class.isAssignableFrom(type)) {
-            final String enumValue = String.valueOf(value);
-            final T[] constants = type.getEnumConstants();
-            if (constants == null) {
-                throw conversionException(type, value);
+            final String stringValue = toString(value);
+
+            try {
+                return type.cast((Enum)Enum.valueOf((Class)type, stringValue));
+            } catch (IllegalArgumentException ex) {
+                // Continue to check fully qualified name.
             }
-            for (final T candidate : constants) {
-                if (((Enum)candidate).name().equalsIgnoreCase(enumValue)) {
-                    return candidate;
+
+            Matcher matcher = ENUM_PATTERN.matcher(stringValue);
+
+            if (!matcher.matches()) {
+                throw new IllegalArgumentException("Value does not follow Java naming conventions, expecting input " +
+                    "like: java.time.DayOfWeek.MONDAY");

Review comment:
       No need to break up the string.
   

##########
File path: src/main/java/org/apache/commons/beanutils2/converters/DurationConverter.java
##########
@@ -74,10 +84,21 @@ public DurationConverter(final Object defaultValue) {
     @Override
     protected <T> T convertToType(final Class<T> type, final Object value) throws Throwable {
         if (Duration.class.equals(type)) {
-            return type.cast(Duration.parse((String.valueOf(value))));
+            final String stringValue = toString(value);
+
+            try {
+                return type.cast(Duration.parse(stringValue));
+            } catch (DateTimeParseException ex) {
+                try {
+                    long number = Long.parseLong(stringValue);
+                    return type.cast(Duration.of(number, ChronoUnit.MILLIS));
+                } catch (RuntimeException rEx) {

Review comment:
       Be more specific (see above). You don't want to catch things like `IllegalThreadStateException`...

##########
File path: src/main/java/org/apache/commons/beanutils2/converters/PeriodConverter.java
##########
@@ -74,7 +87,19 @@ public PeriodConverter(final Object defaultValue) {
     @Override
     protected <T> T convertToType(final Class<T> type, final Object value) throws Throwable {
         if (Period.class.equals(type)) {
-            return type.cast(Period.parse((String.valueOf(value))));
+            final String stringValue = toString(value);
+
+            try {
+                return type.cast(Period.parse(stringValue));
+            } catch (DateTimeParseException ex) {
+                try {
+                    int number = Integer.parseInt(stringValue);
+                    return type.cast(Period.ofDays(number));
+                } catch (RuntimeException rEx) {

Review comment:
       Be more specific.
   

##########
File path: src/main/java/org/apache/commons/beanutils2/converters/InstantConverter.java
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.beanutils2.converters;
+
+import java.time.Instant;
+
+/**
+ * {@link DateTimeConverter} implementation that handles conversion to
+ * and from <b>java.time.LocalDateTime</b> objects.
+ * <p>
+ * This implementation can be configured to handle conversion either
+ * by using a Locale's default format or by specifying a set of format
+ * patterns (note, there is no default String conversion for Calendar).
+ * See the {@link DateTimeConverter} documentation for further details.
+ * </p>
+ * <p>
+ * Can be configured to either return a <i>default value</i> or throw a
+ * {@code ConversionException} if a conversion error occurs.
+ * </p>
+ *
+ * @since 2.0
+ * @see Instant
+ */
+public final class InstantConverter extends DateTimeConverter {
+
+    /**
+     * Construct a <b>java.time.LocalDateTime</b> <i>Converter</i> that throws

Review comment:
       "Construct" -> "Constructs"
   

##########
File path: src/main/java/org/apache/commons/beanutils2/converters/EnumConverter.java
##########
@@ -71,21 +79,42 @@ public EnumConverter(final Object defaultValue) {
      */
     @SuppressWarnings({ "rawtypes" })
     @Override
-    protected <T> T  convertToType(final Class<T> type, final Object value) throws Throwable {
+    protected <T> T convertToType(final Class<T> type, final Object value) throws Throwable {
         if (Enum.class.isAssignableFrom(type)) {
-            final String enumValue = String.valueOf(value);
-            final T[] constants = type.getEnumConstants();
-            if (constants == null) {
-                throw conversionException(type, value);
+            final String stringValue = toString(value);
+
+            try {
+                return type.cast((Enum)Enum.valueOf((Class)type, stringValue));
+            } catch (IllegalArgumentException ex) {
+                // Continue to check fully qualified name.
             }
-            for (final T candidate : constants) {
-                if (((Enum)candidate).name().equalsIgnoreCase(enumValue)) {
-                    return candidate;
+
+            Matcher matcher = ENUM_PATTERN.matcher(stringValue);
+
+            if (!matcher.matches()) {
+                throw new IllegalArgumentException("Value does not follow Java naming conventions, expecting input " +
+                    "like: java.time.DayOfWeek.MONDAY");
+            }
+
+            try {
+                String className = matcher.group(1) + "." + matcher.group(2);
+
+                Class classForName = Class.forName(className);
+
+                if (!classForName.isEnum()) {
+                    throw new IllegalArgumentException("Value provided isn't an enumerated type.");
+                }
+
+                if (!type.isAssignableFrom(classForName)) {
+                    throw new IllegalArgumentException("Class provided is not the required type.");
                 }
+
+                return type.cast((Enum)Enum.valueOf(classForName, matcher.group(3)));
+            } catch (ClassNotFoundException ex) {
+                throw new IllegalArgumentException("Class specified doesn't exist.", ex);

Review comment:
       How about specifying the class name in the message?

##########
File path: src/test/java/org/apache/commons/beanutils2/converters/CharacterConverterTestCase.java
##########
@@ -18,109 +18,97 @@
 
 import org.apache.commons.beanutils2.ConversionException;
 import org.apache.commons.beanutils2.Converter;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
 
-import junit.framework.TestCase;
-import junit.framework.TestSuite;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 
 /**
  * Test Case for the CharacterConverter class.
- *
  */
-public class CharacterConverterTestCase extends TestCase {
-
-    /**
-     * Create Test Suite
-     * @return test suite
-     */
-    public static TestSuite suite() {
-        return new TestSuite(CharacterConverterTestCase.class);
-    }
-
+public class CharacterConverterTestCase {
 
+    private CharacterConverter converter;
 
-    /**
-     * Constructs a new Character Converter test case.
-     * @param name Test Name
-     */
-    public CharacterConverterTestCase(final String name) {
-        super(name);
-    }
-
-    /** Sets Up */
-    @Override
-    public void setUp() throws Exception {
-    }
-
-    /** Tear Down */
-    @Override
-    public void tearDown() throws Exception {
+    @Before
+    public void before() {
+        converter = new CharacterConverter();
     }
 
-
-
     /**
      * Tests whether the primitive char class can be passed as target type.
      */
+    @Test
     public void testConvertToChar() {
-        final Converter converter = new CharacterConverter();
-        assertEquals("Wrong result", new Character('F'), converter.convert(Character.TYPE, "FOO"));
+        assertEquals("Wrong result", Character.valueOf('F'), converter.convert(Character.TYPE, "FOO"));
     }
 
     /**
      * Test Conversion to Character
      */
+    @Test
     public void testConvertToCharacter() {
-        final Converter converter = new CharacterConverter();
-        assertEquals("Character Test", new Character('N'), converter.convert(Character.class, new Character('N')));
-        assertEquals("String Test",    new Character('F'), converter.convert(Character.class, "FOO"));
-        assertEquals("Integer Test",   new Character('3'), converter.convert(Character.class, new Integer(321)));
+        assertEquals("Character Test", Character.valueOf('N'), converter.convert(Character.class, 'N'));

Review comment:
       Good catch on valueOf! :-)

##########
File path: src/test/java/org/apache/commons/beanutils2/converters/PeriodConverterTestCase.java
##########
@@ -86,31 +59,50 @@ public void testSimpleConversion() throws Exception {
         };
 
         final Period[] expected = {
-                Period.parse("P2Y"),
-                Period.parse("P5D"),
-                Period.parse("P1Y2M3D")
+            Period.parse("P2Y"),
+            Period.parse("P5D"),
+            Period.parse("P1Y2M3D")
         };
 
-        for(int i=0;i<expected.length;i++) {
-            assertEquals(message[i] + " to URI",expected[i],converter.convert(Period.class,input[i]));
-            assertEquals(message[i] + " to null type",expected[i],converter.convert(null,input[i]));
+        for (int i = 0; i < expected.length; i++) {
+            assertEquals(message[i] + " to URI", expected[i], converter.convert(Period.class, input[i]));
+            assertEquals(message[i] + " to null type", expected[i], converter.convert(null, input[i]));
         }
 
-        for(int i=0;i<expected.length;i++) {
+        for (int i = 0; i < expected.length; i++) {
             assertEquals(input[i] + " to String", input[i], converter.convert(String.class, expected[i]));
         }
     }
 
     /**
      * Tests a conversion to an unsupported type.
      */
+    @Test(expected = ConversionException.class)
     public void testUnsupportedType() {
-        try {
-            converter.convert(Integer.class, "http://www.apache.org");
-            fail("Unsupported type could be converted!");
-        } catch (final ConversionException cex) {
-            // expected result
-        }
+        converter.convert(Integer.class, "http://www.apache.org");
+    }
+
+    @Test
+    public void testConvertingDefault() {
+        final Period expected = Period.ofDays(2);

Review comment:
       Both of these locals can be inlined.
   

##########
File path: src/test/java/org/apache/commons/beanutils2/converters/CharacterConverterTestCase.java
##########
@@ -18,109 +18,97 @@
 
 import org.apache.commons.beanutils2.ConversionException;
 import org.apache.commons.beanutils2.Converter;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
 
-import junit.framework.TestCase;
-import junit.framework.TestSuite;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 
 /**
  * Test Case for the CharacterConverter class.
- *
  */
-public class CharacterConverterTestCase extends TestCase {
-
-    /**
-     * Create Test Suite
-     * @return test suite
-     */
-    public static TestSuite suite() {
-        return new TestSuite(CharacterConverterTestCase.class);
-    }
-
+public class CharacterConverterTestCase {
 
+    private CharacterConverter converter;
 
-    /**
-     * Constructs a new Character Converter test case.
-     * @param name Test Name
-     */
-    public CharacterConverterTestCase(final String name) {
-        super(name);
-    }
-
-    /** Sets Up */
-    @Override
-    public void setUp() throws Exception {
-    }
-
-    /** Tear Down */
-    @Override
-    public void tearDown() throws Exception {
+    @Before
+    public void before() {
+        converter = new CharacterConverter();
     }
 
-
-
     /**
      * Tests whether the primitive char class can be passed as target type.
      */
+    @Test
     public void testConvertToChar() {
-        final Converter converter = new CharacterConverter();
-        assertEquals("Wrong result", new Character('F'), converter.convert(Character.TYPE, "FOO"));
+        assertEquals("Wrong result", Character.valueOf('F'), converter.convert(Character.TYPE, "FOO"));
     }
 
     /**
      * Test Conversion to Character
      */
+    @Test
     public void testConvertToCharacter() {
-        final Converter converter = new CharacterConverter();
-        assertEquals("Character Test", new Character('N'), converter.convert(Character.class, new Character('N')));
-        assertEquals("String Test",    new Character('F'), converter.convert(Character.class, "FOO"));
-        assertEquals("Integer Test",   new Character('3'), converter.convert(Character.class, new Integer(321)));
+        assertEquals("Character Test", Character.valueOf('N'), converter.convert(Character.class, 'N'));
+        assertEquals("String Test", Character.valueOf('F'), converter.convert(Character.class, "FOO"));
+        assertEquals("Integer Test", Character.valueOf('3'), converter.convert(Character.class, 321));
     }
 
     /**
      * Tests a conversion to character for null input if no default value is
      * provided.
      */
+    @Test(expected = ConversionException.class)
     public void testConvertToCharacterNullNoDefault() {
-        final Converter converter = new CharacterConverter();
-        try {
-            converter.convert(Character.class, null);
-            fail("Expected a ConversionException for null value");
-        } catch (final Exception e) {
-            // expected result
-        }
+        converter.convert(Character.class, null);
     }
 
     /**
      * Test Conversion to String
      */
+    @Test
     public void testConvertToString() {
-
-        final Converter converter = new CharacterConverter();
-
-        assertEquals("Character Test", "N", converter.convert(String.class, new Character('N')));
-        assertEquals("String Test",    "F", converter.convert(String.class, "FOO"));
-        assertEquals("Integer Test",   "3", converter.convert(String.class, new Integer(321)));
-        assertEquals("Null Test",     null, converter.convert(String.class, null));
+        assertEquals("Character Test", "N", converter.convert(String.class, 'N'));
+        assertEquals("String Test", "F", converter.convert(String.class, "FOO"));
+        assertEquals("Integer Test", "3", converter.convert(String.class, 321));
+        assertNull("Null Test", converter.convert(String.class, null));
     }
 
     /**
      * Tries a conversion to an unsupported type.
      */
+    @Test(expected = ConversionException.class)
     public void testConvertToUnsupportedType() {
-        final Converter converter = new CharacterConverter();
-        try {
-            converter.convert(Integer.class, "Test");
-            fail("Could convert to unsupported type!");
-        } catch (final ConversionException cex) {
-            // expected result
-        }
+        converter.convert(Integer.class, "Test");
     }
 
     /**
      * Test Conversion to Character (with default)
      */
+    @Test
     public void testDefault() {
         final Converter converter = new CharacterConverter("C");
-        assertEquals("Default Test",   new Character('C'), converter.convert(Character.class, null));
+        assertEquals("Default Test", Character.valueOf('C'), converter.convert(Character.class, null));
+    }
+
+    /**
+     * If a hexadecimal value is provided, we'll convert it to a
+     * a character if possible.
+     */
+    @Test
+    public void testConvertHexCharacter() {
+        final char expected = 'A';

Review comment:
       Inline local vars.
   

##########
File path: src/main/java/org/apache/commons/beanutils2/converters/PeriodConverter.java
##########
@@ -74,7 +87,19 @@ public PeriodConverter(final Object defaultValue) {
     @Override
     protected <T> T convertToType(final Class<T> type, final Object value) throws Throwable {
         if (Period.class.equals(type)) {
-            return type.cast(Period.parse((String.valueOf(value))));
+            final String stringValue = toString(value);
+
+            try {
+                return type.cast(Period.parse(stringValue));
+            } catch (DateTimeParseException ex) {
+                try {
+                    int number = Integer.parseInt(stringValue);

Review comment:
       Inline `number`.
   

##########
File path: src/test/java/org/apache/commons/beanutils2/converters/CharacterConverterTestCase.java
##########
@@ -18,109 +18,97 @@
 
 import org.apache.commons.beanutils2.ConversionException;
 import org.apache.commons.beanutils2.Converter;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
 
-import junit.framework.TestCase;
-import junit.framework.TestSuite;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 
 /**
  * Test Case for the CharacterConverter class.
- *
  */
-public class CharacterConverterTestCase extends TestCase {
-
-    /**
-     * Create Test Suite
-     * @return test suite
-     */
-    public static TestSuite suite() {
-        return new TestSuite(CharacterConverterTestCase.class);
-    }
-
+public class CharacterConverterTestCase {
 
+    private CharacterConverter converter;
 
-    /**
-     * Constructs a new Character Converter test case.
-     * @param name Test Name
-     */
-    public CharacterConverterTestCase(final String name) {
-        super(name);
-    }
-
-    /** Sets Up */
-    @Override
-    public void setUp() throws Exception {
-    }
-
-    /** Tear Down */
-    @Override
-    public void tearDown() throws Exception {
+    @Before
+    public void before() {
+        converter = new CharacterConverter();
     }
 
-
-
     /**
      * Tests whether the primitive char class can be passed as target type.
      */
+    @Test
     public void testConvertToChar() {
-        final Converter converter = new CharacterConverter();
-        assertEquals("Wrong result", new Character('F'), converter.convert(Character.TYPE, "FOO"));
+        assertEquals("Wrong result", Character.valueOf('F'), converter.convert(Character.TYPE, "FOO"));
     }
 
     /**
      * Test Conversion to Character
      */
+    @Test
     public void testConvertToCharacter() {
-        final Converter converter = new CharacterConverter();
-        assertEquals("Character Test", new Character('N'), converter.convert(Character.class, new Character('N')));
-        assertEquals("String Test",    new Character('F'), converter.convert(Character.class, "FOO"));
-        assertEquals("Integer Test",   new Character('3'), converter.convert(Character.class, new Integer(321)));
+        assertEquals("Character Test", Character.valueOf('N'), converter.convert(Character.class, 'N'));
+        assertEquals("String Test", Character.valueOf('F'), converter.convert(Character.class, "FOO"));
+        assertEquals("Integer Test", Character.valueOf('3'), converter.convert(Character.class, 321));
     }
 
     /**
      * Tests a conversion to character for null input if no default value is
      * provided.
      */
+    @Test(expected = ConversionException.class)
     public void testConvertToCharacterNullNoDefault() {
-        final Converter converter = new CharacterConverter();
-        try {
-            converter.convert(Character.class, null);
-            fail("Expected a ConversionException for null value");
-        } catch (final Exception e) {
-            // expected result
-        }
+        converter.convert(Character.class, null);
     }
 
     /**
      * Test Conversion to String
      */
+    @Test
     public void testConvertToString() {
-
-        final Converter converter = new CharacterConverter();
-
-        assertEquals("Character Test", "N", converter.convert(String.class, new Character('N')));
-        assertEquals("String Test",    "F", converter.convert(String.class, "FOO"));
-        assertEquals("Integer Test",   "3", converter.convert(String.class, new Integer(321)));
-        assertEquals("Null Test",     null, converter.convert(String.class, null));
+        assertEquals("Character Test", "N", converter.convert(String.class, 'N'));
+        assertEquals("String Test", "F", converter.convert(String.class, "FOO"));
+        assertEquals("Integer Test", "3", converter.convert(String.class, 321));
+        assertNull("Null Test", converter.convert(String.class, null));
     }
 
     /**
      * Tries a conversion to an unsupported type.
      */
+    @Test(expected = ConversionException.class)
     public void testConvertToUnsupportedType() {
-        final Converter converter = new CharacterConverter();
-        try {
-            converter.convert(Integer.class, "Test");
-            fail("Could convert to unsupported type!");
-        } catch (final ConversionException cex) {
-            // expected result
-        }
+        converter.convert(Integer.class, "Test");
     }
 
     /**
      * Test Conversion to Character (with default)
      */
+    @Test
     public void testDefault() {
         final Converter converter = new CharacterConverter("C");
-        assertEquals("Default Test",   new Character('C'), converter.convert(Character.class, null));
+        assertEquals("Default Test", Character.valueOf('C'), converter.convert(Character.class, null));
+    }
+
+    /**
+     * If a hexadecimal value is provided, we'll convert it to a
+     * a character if possible.
+     */
+    @Test
+    public void testConvertHexCharacter() {
+        final char expected = 'A';
+        final char actual = converter.convert(Character.class, "0x41");
+
+        Assert.assertEquals(expected, actual);
+    }
+
+    @Test
+    public void testConvertHexSymbol() {
+        final char expected = '£';

Review comment:
       Inline local vars.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org