Map doesn't implement collection. And the Iterator::hasNext is called on a new Iterator obtained from the Iterable. It was implemented as a Collection before, but I had a thought and implemented it as such. The javadocs for Iterable don't exactly state whether or not the Iterator is a singleton, but that would probably break something if used like that anyways.
On 6 January 2017 at 22:37, Remko Popma <remko.po...@gmail.com> wrote: > Instead of Map::isEmpty, we could do Collection::isEmpty. Also is it a > good idea to call Iterator::hasNext? In some implementations calling this > method moves the cursor one position forward. Do we actually need to check > that an Iterator is empty anywhere? > > Sent from my iPhone > > > On Jan 7, 2017, at 12:51, mattsic...@apache.org wrote: > > > > Repository: logging-log4j2 > > Updated Branches: > > refs/heads/master b0daba63f -> 61b3ab301 > > > > > > Extract isEmpty() logic from RequiredValidator > > > > > > Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo > > Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/ > commit/61b3ab30 > > Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/ > 61b3ab30 > > Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/ > 61b3ab30 > > > > Branch: refs/heads/master > > Commit: 61b3ab301a4d54c28e736ca3a37f984475969377 > > Parents: b0daba6 > > Author: Matt Sicker <matt.sic...@spr.com> > > Authored: Fri Jan 6 21:50:49 2017 -0600 > > Committer: Matt Sicker <matt.sic...@spr.com> > > Committed: Fri Jan 6 21:50:49 2017 -0600 > > > > ---------------------------------------------------------------------- > > .../validators/RequiredValidator.java | 24 +------ > > .../apache/logging/log4j/core/util/Assert.java | 46 +++++++++++++ > > .../logging/log4j/core/util/AssertTest.java | 68 > ++++++++++++++++++++ > > 3 files changed, 116 insertions(+), 22 deletions(-) > > ---------------------------------------------------------------------- > > > > > > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ > 61b3ab30/log4j-core/src/main/java/org/apache/logging/log4j/ > core/config/plugins/validation/validators/RequiredValidator.java > > ---------------------------------------------------------------------- > > diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/ > config/plugins/validation/validators/RequiredValidator.java > b/log4j-core/src/main/java/org/apache/logging/log4j/core/ > config/plugins/validation/validators/RequiredValidator.java > > index 5e6cec0..98c0a71 100644 > > --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/ > config/plugins/validation/validators/RequiredValidator.java > > +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/ > config/plugins/validation/validators/RequiredValidator.java > > @@ -22,6 +22,7 @@ import java.util.Map; > > import org.apache.logging.log4j.Logger; > > import org.apache.logging.log4j.core.config.plugins.validation. > ConstraintValidator; > > import org.apache.logging.log4j.core.config.plugins.validation. > constraints.Required; > > +import org.apache.logging.log4j.core.util.Assert; > > import org.apache.logging.log4j.status.StatusLogger; > > > > /** > > @@ -49,28 +50,7 @@ public class RequiredValidator implements > ConstraintValidator<Required> { > > > > @Override > > public boolean isValid(final String name, final Object value) { > > - if (value == null) { > > - return err(name); > > - } > > - if (value instanceof CharSequence) { > > - final CharSequence sequence = (CharSequence) value; > > - return sequence.length() != 0 || err(name); > > - } > > - final Class<?> clazz = value.getClass(); > > - if (clazz.isArray()) { > > - final Object[] array = (Object[]) value; > > - return array.length != 0 || err(name); > > - } > > - if (Collection.class.isAssignableFrom(clazz)) { > > - final Collection<?> collection = (Collection<?>) value; > > - return collection.size() != 0 || err(name); > > - } > > - if (Map.class.isAssignableFrom(clazz)) { > > - final Map<?, ?> map = (Map<?, ?>) value; > > - return map.size() != 0 || err(name); > > - } > > - // LOGGER.debug("Encountered type [{}] which can only be > checked for null.", clazz.getName()); > > - return true; > > + return Assert.isNonEmpty(value) || err(name); > > } > > > > private boolean err(final String name) { > > > > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ > 61b3ab30/log4j-core/src/main/java/org/apache/logging/log4j/ > core/util/Assert.java > > ---------------------------------------------------------------------- > > diff --git > > a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Assert.java > b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Assert.java > > index e37090f..d46c06a 100644 > > --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/ > util/Assert.java > > +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/ > util/Assert.java > > @@ -16,6 +16,8 @@ > > */ > > package org.apache.logging.log4j.core.util; > > > > +import java.util.Map; > > + > > /** > > * Utility class providing common validation logic. > > */ > > @@ -23,6 +25,50 @@ public final class Assert { > > private Assert() { > > } > > > > + /** > > + * Checks if an object has empty semantics. The following scenarios > are considered empty: > > + * <ul> > > + * <li>{@code null}</li> > > + * <li>empty {@link CharSequence}</li> > > + * <li>empty array</li> > > + * <li>empty {@link Iterable}</li> > > + * <li>empty {@link Map}</li> > > + * </ul> > > + * > > + * @param o value to check for emptiness > > + * @return true if the value is empty, false otherwise > > + * @since 2.8 > > + */ > > + public static boolean isEmpty(final Object o) { > > + if (o == null) { > > + return true; > > + } > > + if (o instanceof CharSequence) { > > + return ((CharSequence) o).length() == 0; > > + } > > + if (o.getClass().isArray()) { > > + return ((Object[]) o).length == 0; > > + } > > + if (o instanceof Iterable) { > > + return !((Iterable<?>) o).iterator().hasNext(); > > + } > > + if (o instanceof Map) { > > + return ((Map<?, ?>) o).isEmpty(); > > + } > > + return false; > > + } > > + > > + /** > > + * Opposite of {@link #isEmpty(Object)}. > > + * > > + * @param o value to check for non-emptiness > > + * @return true if the value is non-empty, false otherwise > > + * @since 2.8 > > + */ > > + public static boolean isNonEmpty(final Object o) { > > + return !isEmpty(o); > > + } > > + > > public static int valueIsAtLeast(final int value, final int > minValue) { > > if (value < minValue) { > > throw new IllegalArgumentException("Value should be at > least " + minValue + " but was " + value); > > > > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ > 61b3ab30/log4j-core/src/test/java/org/apache/logging/log4j/ > core/util/AssertTest.java > > ---------------------------------------------------------------------- > > diff --git > > a/log4j-core/src/test/java/org/apache/logging/log4j/core/util/AssertTest.java > b/log4j-core/src/test/java/org/apache/logging/log4j/core/ > util/AssertTest.java > > new file mode 100644 > > index 0000000..242c41e > > --- /dev/null > > +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/ > util/AssertTest.java > > @@ -0,0 +1,68 @@ > > +/* > > + * 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.logging.log4j.core.util; > > + > > +import java.util.ArrayList; > > +import java.util.Collections; > > +import java.util.HashMap; > > + > > +import org.junit.Test; > > +import org.junit.runner.RunWith; > > +import org.junit.runners.Parameterized; > > + > > +import static org.junit.Assert.*; > > + > > +/** > > + * > > + */ > > +@RunWith(Parameterized.class) > > +public class AssertTest { > > + > > + private final Object value; > > + private final boolean isEmpty; > > + > > + @Parameterized.Parameters > > + public static Object[][] data() { > > + return new Object[][]{ > > + // value, isEmpty > > + {null, true}, > > + {"", true}, > > + {new Object[0], true}, > > + {new ArrayList<>(), true}, > > + {new HashMap<>(), true}, > > + {0, false}, > > + {1, false}, > > + {false, false}, > > + {true, false}, > > + {new Object[]{null}, false}, > > + {Collections.singletonList(null), false}, > > + {Collections.singletonMap("", null), false}, > > + {"null", false} > > + }; > > + } > > + > > + public AssertTest(final Object value, final boolean isEmpty) { > > + this.value = value; > > + this.isEmpty = isEmpty; > > + } > > + > > + @Test > > + public void isEmpty() throws Exception { > > + assertEquals(isEmpty, Assert.isEmpty(value)); > > + } > > + > > +} > > \ No newline at end of file > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org > For additional commands, e-mail: log4j-dev-h...@logging.apache.org > > -- Matt Sicker <boa...@gmail.com>
