Repository: any23 Updated Branches: refs/heads/master e51e38dac -> 5b93f21ec
add convenience factory methods to Setting Project: http://git-wip-us.apache.org/repos/asf/any23/repo Commit: http://git-wip-us.apache.org/repos/asf/any23/commit/5b93f21e Tree: http://git-wip-us.apache.org/repos/asf/any23/tree/5b93f21e Diff: http://git-wip-us.apache.org/repos/asf/any23/diff/5b93f21e Branch: refs/heads/master Commit: 5b93f21ec89eea437b82812616aa05fb13d42bcb Parents: e51e38d Author: Hans <[email protected]> Authored: Sat Oct 27 15:23:31 2018 -0500 Committer: Hans <[email protected]> Committed: Sat Oct 27 15:23:31 2018 -0500 ---------------------------------------------------------------------- .../org/apache/any23/configuration/Setting.java | 152 ++++++++++++++++--- .../any23/configuration/SettingsTest.java | 54 +++++-- .../main/java/org/apache/any23/cli/Rover.java | 4 +- .../org/apache/any23/writer/WriterSettings.java | 4 +- 4 files changed, 183 insertions(+), 31 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/any23/blob/5b93f21e/api/src/main/java/org/apache/any23/configuration/Setting.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/org/apache/any23/configuration/Setting.java b/api/src/main/java/org/apache/any23/configuration/Setting.java index 25f07f8..0fc7919 100644 --- a/api/src/main/java/org/apache/any23/configuration/Setting.java +++ b/api/src/main/java/org/apache/any23/configuration/Setting.java @@ -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; } http://git-wip-us.apache.org/repos/asf/any23/blob/5b93f21e/api/src/test/java/org/apache/any23/configuration/SettingsTest.java ---------------------------------------------------------------------- diff --git a/api/src/test/java/org/apache/any23/configuration/SettingsTest.java b/api/src/test/java/org/apache/any23/configuration/SettingsTest.java index 4e799d4..80aef91 100644 --- a/api/src/test/java/org/apache/any23/configuration/SettingsTest.java +++ b/api/src/test/java/org/apache/any23/configuration/SettingsTest.java @@ -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) { http://git-wip-us.apache.org/repos/asf/any23/blob/5b93f21e/cli/src/main/java/org/apache/any23/cli/Rover.java ---------------------------------------------------------------------- diff --git a/cli/src/main/java/org/apache/any23/cli/Rover.java b/cli/src/main/java/org/apache/any23/cli/Rover.java index c2f0557..8556669 100644 --- a/cli/src/main/java/org/apache/any23/cli/Rover.java +++ b/cli/src/main/java/org/apache/any23/cli/Rover.java @@ -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() { http://git-wip-us.apache.org/repos/asf/any23/blob/5b93f21e/core/src/main/java/org/apache/any23/writer/WriterSettings.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/any23/writer/WriterSettings.java b/core/src/main/java/org/apache/any23/writer/WriterSettings.java index 44094ad..2d3aa47 100644 --- a/core/src/main/java/org/apache/any23/writer/WriterSettings.java +++ b/core/src/main/java/org/apache/any23/writer/WriterSettings.java @@ -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); }
