add convenience factory methods to Setting
authorHans <firedrake93@gmail.com>
Sat, 27 Oct 2018 20:23:31 +0000 (15:23 -0500)
committerHans <firedrake93@gmail.com>
Sat, 27 Oct 2018 20:23:31 +0000 (15:23 -0500)
api/src/main/java/org/apache/any23/configuration/Setting.java
api/src/test/java/org/apache/any23/configuration/SettingsTest.java
cli/src/main/java/org/apache/any23/cli/Rover.java
core/src/main/java/org/apache/any23/writer/WriterSettings.java

index 25f07f8..0fc7919 100644 (file)
@@ -33,7 +33,7 @@ import java.util.regex.Pattern;
  */
 public abstract class Setting<V> implements Cloneable {
 
-    private final Key<V> key;
+    private final Key key;
     private V value;
 
     /**
@@ -45,7 +45,35 @@ public abstract class Setting<V> implements Cloneable {
      */
     protected Setting(String identifier, V defaultValue) {
         checkIdentifier(identifier);
-        this.key = new Key<>(identifier, lookupValueType(getClass(), identifier), defaultValue);
+        this.key = new Key(identifier, lookupValueType(getClass(), identifier), defaultValue != null);
+        this.value = defaultValue;
+    }
+
+    /**
+     * Constructs a new setting with the specified identifier, value type, and default value.
+     * @param identifier the identifier for this setting
+     * @param valueType the value type for this setting
+     * @param defaultValue the default value for this setting
+     * @throws IllegalArgumentException if the identifier is invalid, or
+     * the value type is primitive, mutable, or has type parameters
+     */
+    protected Setting(String identifier, Class<V> valueType, V defaultValue) {
+        this(identifier, defaultValue, valueType);
+        if (valueType.isArray()) {
+            throw new IllegalArgumentException(identifier + " value class must be immutable");
+        } else if (valueType.getTypeParameters().length != 0) {
+            throw new IllegalArgumentException(identifier + " setting key must fill in type parameters for "
+                    + valueType.toGenericString());
+        } else if (valueType.isPrimitive()) {
+            //ensure using primitive wrapper classes
+            //so that Class.isInstance(), etc. will work as expected
+            throw new IllegalArgumentException(identifier + " value class cannot be primitive");
+        }
+    }
+
+    private Setting(String identifier, V defaultValue, Class<V> valueType) {
+        checkIdentifier(identifier);
+        this.key = new Key(identifier, valueType, defaultValue != null);
         this.value = defaultValue;
     }
 
@@ -58,13 +86,14 @@ public abstract class Setting<V> implements Cloneable {
 
     /**
      * Subclasses may override this method to check that new values for this setting are valid.
-     * The default implementation of this method throws a {@link NullPointerException} if the new value is null and the default value is non-null.
+     * The default implementation of this method throws a {@link NullPointerException} if the
+     * new value is null and the original default value for this setting was non-null.
      *
      * @param newValue the new value for this setting
      * @throws Exception if the new value for this setting is invalid
      */
     protected void checkValue(V newValue) throws Exception {
-        if (newValue == null && key.defaultValue != null) {
+        if (newValue == null && key.nonnull) {
             throw new NullPointerException();
         }
     }
@@ -77,13 +106,6 @@ public abstract class Setting<V> implements Cloneable {
     }
 
     /**
-     * @return the default value for this setting
-     */
-    public final V getDefaultValue() {
-        return key.defaultValue;
-    }
-
-    /**
      * @return the type of value supported for this setting
      */
     public final Type getValueType() {
@@ -146,20 +168,114 @@ public abstract class Setting<V> implements Cloneable {
     }
 
 
+    /**
+     * Convenience method to create a new setting with the specified identifier and default value.
+     * @param identifier the identifier for this setting
+     * @param defaultValue the default value for this setting
+     * @return the new setting
+     * @throws IllegalArgumentException if the identifier is invalid
+     */
+    public static Setting<Boolean> create(String identifier, Boolean defaultValue) {
+        return new Impl<>(identifier, defaultValue, Boolean.class);
+    }
+
+    /**
+     * Convenience method to create a new setting with the specified identifier and default value.
+     * @param identifier the identifier for this setting
+     * @param defaultValue the default value for this setting
+     * @return the new setting
+     * @throws IllegalArgumentException if the identifier is invalid
+     */
+    public static Setting<String> create(String identifier, String defaultValue) {
+        return new Impl<>(identifier, defaultValue, String.class);
+    }
+
+    /**
+     * Convenience method to create a new setting with the specified identifier and default value.
+     * @param identifier the identifier for this setting
+     * @param defaultValue the default value for this setting
+     * @return the new setting
+     * @throws IllegalArgumentException if the identifier is invalid
+     */
+    public static Setting<Integer> create(String identifier, Integer defaultValue) {
+        return new Impl<>(identifier, defaultValue, Integer.class);
+    }
+
+    /**
+     * Convenience method to create a new setting with the specified identifier and default value.
+     * @param identifier the identifier for this setting
+     * @param defaultValue the default value for this setting
+     * @return the new setting
+     * @throws IllegalArgumentException if the identifier is invalid
+     */
+    public static Setting<Long> create(String identifier, Long defaultValue) {
+        return new Impl<>(identifier, defaultValue, Long.class);
+    }
+
+    /**
+     * Convenience method to create a new setting with the specified identifier and default value.
+     * @param identifier the identifier for this setting
+     * @param defaultValue the default value for this setting
+     * @return the new setting
+     * @throws IllegalArgumentException if the identifier is invalid
+     */
+    public static Setting<Float> create(String identifier, Float defaultValue) {
+        return new Impl<>(identifier, defaultValue, Float.class);
+    }
+
+    /**
+     * Convenience method to create a new setting with the specified identifier and default value.
+     * @param identifier the identifier for this setting
+     * @param defaultValue the default value for this setting
+     * @return the new setting
+     * @throws IllegalArgumentException if the identifier is invalid
+     */
+    public static Setting<Double> create(String identifier, Double defaultValue) {
+        return new Impl<>(identifier, defaultValue, Double.class);
+    }
+
+    /**
+     * Convenience method to create a new setting with the specified identifier,
+     * value type, and default value.
+     * @param identifier the identifier for this setting
+     * @param valueType the value type for this setting
+     * @param defaultValue the default value for this setting
+     * @return the new setting
+     * @throws IllegalArgumentException if the identifier is invalid, or
+     * the value type is primitive, mutable, or has type parameters
+     */
+    public static <V> Setting<V> create(String identifier, Class<V> valueType, V defaultValue) {
+        return new Impl<>(identifier, valueType, defaultValue);
+    }
+
+
 
     ///////////////////////////////////////
     // Private static helpers
     ///////////////////////////////////////
 
-    private static final class Key<V> {
+    // Use Impl when possible to avoid creating an anonymous class (and class file) for
+    // every single existing setting, and to avoid the overhead of value type lookup.
+    private static class Impl<V> extends Setting<V> {
+        //this constructor does not check the value type
+        private Impl(String identifier, V defaultValue, Class<V> valueType) {
+            super(identifier, defaultValue, valueType);
+        }
+        //this constructor does check the value type
+        private Impl(String identifier, Class<V> valueType, V defaultValue) {
+            super(identifier, valueType, defaultValue);
+        }
+    }
+
+    private static final class Key {
         final String identifier;
         final Type valueType;
-        final V defaultValue;
+        final boolean nonnull;
 
-        Key(String identifier, Type valueType, V defaultValue) {
+        Key(String identifier, Type valueType, boolean nonnull) {
             this.identifier = identifier;
             this.valueType = valueType;
-            this.defaultValue = defaultValue;
+            this.nonnull = nonnull;
         }
     }
 
@@ -207,14 +323,14 @@ public abstract class Setting<V> implements Cloneable {
                         if (((Class) type).isArray()) {
                             throw new IllegalArgumentException(identifier + " value class must be immutable");
                         } else if (((Class) type).getTypeParameters().length != 0) {
-                            throw new IllegalArgumentException(identifier + " setting key must fill in type parameters for " + ((Class) type).toGenericString());
+                            throw new IllegalArgumentException(identifier + " setting must fill in type parameters for " + ((Class) type).toGenericString());
                         }
                     } else if (type instanceof GenericArrayType) {
                         throw new IllegalArgumentException(identifier + " value class must be immutable");
                     } else if (type instanceof TypeVariable) {
-                        throw new IllegalArgumentException("Invalid setting key type 'Key<" + type.getTypeName() + ">' for identifier " + identifier);
+                        throw new IllegalArgumentException("Invalid setting type 'Key<" + type.getTypeName() + ">' for identifier " + identifier);
                     } else if (!(type instanceof ParameterizedType)) {
-                        throw new IllegalArgumentException(identifier + " invalid key type " + type + " (" + type.getClass().getName() + ")");
+                        throw new IllegalArgumentException(identifier + " invalid type " + type + " (" + type.getClass().getName() + ")");
                     }
                     return type;
                 }
index 4e799d4..80aef91 100644 (file)
@@ -39,7 +39,7 @@ public class SettingsTest {
 
     @Test
     public void testNonNullSetting() {
-        Setting<String> nonNull = new Setting<String>("nulltest", "A nonnull string") {};
+        Setting<String> nonNull = Setting.create("nulltest", "A nonnull string");
         try {
             nonNull.withValue(null);
             fail();
@@ -50,15 +50,15 @@ public class SettingsTest {
 
     @Test
     public void testNullableSetting() {
-        Setting<String> nullable = new Setting<String>("nulltest", null) {};
+        Setting<String> nullable = Setting.create("nulltest", (String)null);
         assertNull(nullable.withValue(null).getValue());
     }
 
     @Test
     public void testDuplicateIdentifiers() {
         try {
-            Setting<String> first = new Setting<String>("foo", "") {};
-            Setting<String> second = new Setting<String>("foo", "") {};
+            Setting<String> first = Setting.create("foo", "");
+            Setting<String> second = Setting.create("foo", "");
 
             Settings.of(first, second);
 
@@ -70,7 +70,7 @@ public class SettingsTest {
 
     @Test
     public void testFind() {
-        Setting<String> key = new Setting<String>("foo", "key") {};
+        Setting<String> key = Setting.create("foo", "key");
         Setting<String> element = key.withValue("element");
 
         Settings settings = Settings.of(element);
@@ -87,7 +87,7 @@ public class SettingsTest {
 
     @Test
     public void testGetPresentSetting() {
-        Setting<String> key = new Setting<String>("foo", "key") {};
+        Setting<String> key = Setting.create("foo", "key");
 
         Setting<String> actual = key.withValue("actual");
         Settings settings = Settings.of(actual);
@@ -97,9 +97,9 @@ public class SettingsTest {
 
     @Test
     public void testGetAbsentSetting() {
-        Setting<String> key = new Setting<String>("foo", "key") {};
+        Setting<String> key = Setting.create("foo", "key");
 
-        Setting<String> actual = new Setting<String>("foo", "actual") {};
+        Setting<String> actual = Setting.create("foo", "actual");
         Settings settings = Settings.of(actual);
 
         assertSame(key.getValue(), settings.get(key));
@@ -107,7 +107,7 @@ public class SettingsTest {
 
     @Test
     public void testGetNullSetting() {
-        Setting<String> baseKey = new Setting<String>("foo", null) {};
+        Setting<String> baseKey = Setting.create("foo", (String)null);
 
         Settings settings = Settings.of(baseKey);
         assertNull(settings.get(baseKey.withValue("not null")));
@@ -118,6 +118,7 @@ public class SettingsTest {
 
     @Test
     public void testSettingType() {
+        assertEquals(CharSequence.class, Setting.create("foo", CharSequence.class, "").getValueType());
         assertEquals(CharSequence.class, new Setting<CharSequence>("foo", ""){}.getValueType());
 
         Type mapType = new Setting<Map<String, Integer>>("foo", Collections.emptyMap()){}.getValueType();
@@ -207,6 +208,13 @@ public class SettingsTest {
         }
 
         try {
+            Setting.create(null, 0);
+            fail();
+        } catch (IllegalArgumentException e) {
+            //test passes; ignore
+        }
+
+        try {
             new Setting<Integer>(" ", null) {};
             fail();
         } catch (IllegalArgumentException e) {
@@ -214,6 +222,34 @@ public class SettingsTest {
         }
 
         try {
+            Setting.create(" ", 0);
+            fail();
+        } catch (IllegalArgumentException e) {
+            //test passes; ignore
+        }
+
+        try {
+            Setting.create("foo", List.class, null);
+            fail();
+        } catch (IllegalArgumentException e) {
+            //test passes; ignore
+        }
+
+        try {
+            Setting.create("foo", boolean.class, null);
+            fail();
+        } catch (IllegalArgumentException e) {
+            //test passes; ignore
+        }
+
+        try {
+            Setting.create("foo", Integer[].class, null);
+            fail();
+        } catch (IllegalArgumentException e) {
+            //test passes; ignore
+        }
+
+        try {
             new Setting<Integer[]>("foo", null) {};
             fail();
         } catch (IllegalArgumentException e) {
index c2f0557..8556669 100644 (file)
@@ -77,8 +77,8 @@ public class Rover extends BaseTool {
     private static final String DEFAULT_WRITER_IDENTIFIER = NTriplesWriterFactory.IDENTIFIER;
 
     static {
-        final Setting<Boolean> ALWAYS_SUPPRESS_CSS_TRIPLES = new Setting<Boolean>(
-                "alwayssuppresscsstriples", Boolean.TRUE) {};
+        final Setting<Boolean> ALWAYS_SUPPRESS_CSS_TRIPLES = Setting.create(
+                "alwayssuppresscsstriples", Boolean.TRUE);
         final Settings supportedSettings = Settings.of(ALWAYS_SUPPRESS_CSS_TRIPLES);
 
         registry.register(new DecoratingWriterFactory() {
index 44094ad..2d3aa47 100644 (file)
@@ -46,12 +46,12 @@ public class WriterSettings {
     /**
      * Directive to writer that output should be printed in a way to maximize human readability.
      */
-    public static final Setting<Boolean> PRETTY_PRINT = new Setting<Boolean>("pretty", Boolean.TRUE){};
+    public static final Setting<Boolean> PRETTY_PRINT = Setting.create("pretty", Boolean.TRUE);
 
     /**
      * Directive to writer that at least the non-ASCII characters should be escaped.
      */
-    public static final Setting<Boolean> PRINT_ASCII = new Setting<Boolean>("ascii", Boolean.FALSE){};
+    public static final Setting<Boolean> PRINT_ASCII = Setting.create("ascii", Boolean.FALSE);
 
 
 }