Claudenw commented on code in PR #412:
URL:
https://github.com/apache/commons-configuration/pull/412#discussion_r1590247821
##########
src/main/java/org/apache/commons/configuration2/AbstractConfiguration.java:
##########
@@ -429,6 +443,22 @@ public final boolean containsKey(final String key) {
*/
protected abstract boolean containsKeyInternal(String key);
+ /**
+ * Returns true if this configuration contains one or more matches to this
value. This operation stops at first
+ * match but may be more expensive than the {@link #containsKeyInternal
containsKey} method.
+ * <p>
+ * The implementation of this method will be different depending on the
type of Configuration used.
+ *
+ * <p>Note that this method is identical in functionality to
+ * {@link #containsValue containsValue}, (which is part of the {@link
ImmutableConfiguration} interface).
+ *
+ * @param value a value to search for
+ * @return {@code true} if and only if some key maps to the {@code value}
argument in this hashtable as determined
+ * by the {@code equals} method; {@code false} otherwise.
+ * @throws NullPointerException if the value is {@code null}
+ */
Review Comment:
My approach is: if you are writing a javadoc add the `@since` annotation.
##########
src/main/java/org/apache/commons/configuration2/AbstractConfiguration.java:
##########
@@ -1517,6 +1547,25 @@ protected int sizeInternal() {
return size;
}
+ /**
+ * Checks if the specified value exists in the properties structure mapped
by the provided keys.
+ *
+ * @param keys an Iterator of String keys to search for the value
+ * @param value the String value to search for in the properties
+ * @return true if the value is found in the properties, false otherwise
+ * @throws NullPointerException if keys or value is null
+ */
+ protected boolean contains(Iterator<String> keys, final String value) {
+ Objects.requireNonNull(keys, "Parameter \"keys\" cannot be null");
Review Comment:
See my previous comment re `Objects.requireNonNull()`
##########
src/main/java/org/apache/commons/configuration2/AbstractConfiguration.java:
##########
@@ -419,6 +419,20 @@ public final boolean containsKey(final String key) {
}
}
+ /**
+ * {@inheritDoc} This implementation handles synchronization and delegates
to {@code containsKeyInternal()}.
+ */
+ @Override
+ public final boolean containsValue(final String value) {
+ Objects.requireNonNull(value, "Parameter \"value\" cannot be null");
Review Comment:
I think the `Objects.requiredNull` should be removed since this is an
AbstractConfiguration and my actually be implementing a configuration that
allows nulls. It may make sense in final classes to check for null but not
here.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
