dsmiley commented on code in PR #3048:
URL: https://github.com/apache/solr/pull/3048#discussion_r1934300672
##########
solr/solrj/src/java/org/apache/solr/common/util/NamedList.java:
##########
@@ -238,6 +238,14 @@ public int indexOf(String name, int start) {
return -1;
}
+ /***
+ * Scans the list sequentially beginning at index 0
Review Comment:
nitpick: javadoc should include clarity that it's the index of the name.
It's not great to require looking at a parameter's name to disambiguate the
meaning of a method.
##########
solr/solrj/src/java/org/apache/solr/common/util/NamedList.java:
##########
@@ -403,8 +411,8 @@ public Map<String, T> asShallowMap() {
return asShallowMap(false);
}
- public Map<String, T> asShallowMap(boolean allowDps) {
- return new Map<String, T>() {
+ private Map<String, T> asShallowMap(boolean allowDps) {
Review Comment:
now's not the time to make private but feel free to deprecate.
##########
solr/solrj/src/java/org/apache/solr/common/util/NamedList.java:
##########
@@ -845,7 +853,7 @@ public void forEachKey(Consumer<String> fun) {
}
}
- public void forEach(BiConsumer<String, ? super T> action) {
+ public void forEach(BiConsumer<? super String, ? super T> action) {
Review Comment:
did doing this fix something? it's weird.
##########
solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java:
##########
@@ -68,24 +75,123 @@ public SimpleOrderedMap(Map.Entry<String, T>[]
nameValuePairs) {
public SimpleOrderedMap<T> clone() {
ArrayList<Object> newList = new ArrayList<>(nvPairs.size());
newList.addAll(nvPairs);
- return new SimpleOrderedMap<>(newList);
+ return new SimpleOrderedMap<T>(newList);
+ }
+
+ @Override
+ public boolean isEmpty() {
+ return nvPairs.isEmpty();
+ }
+
+ @Override
+ public boolean containsKey(final Object key) {
+ return this.indexOf((String) key) >= 0;
+ }
+
+ @Override
+ public boolean containsValue(final Object value) {
+ return values().contains(value);
}
/**
- * Returns a shared, empty, and immutable instance of SimpleOrderedMap.
+ * {@inheritDoc}
+ *
+ * <p>Has linear lookup time O(N)
*
- * @return Empty SimpleOrderedMap (immutable)
+ * @see NamedList#get(String)
*/
- public static SimpleOrderedMap<Object> of() {
- return EMPTY;
+ @Override
+ public T get(final Object key) {
+ return super.get((String) key);
+ }
+
+ @Override
+ public T put(String key, T value) {
+ int idx = indexOf(key);
+ if (idx == -1) {
+ add(key, value);
+ return null;
+ } else {
+ T t = get(key);
+ setVal(idx, value);
+ return t;
+ }
}
/**
- * Returns an immutable instance of SimpleOrderedMap with a single key-value
pair.
+ * @see NamedList#remove(String)
+ */
+ @Override
+ public T remove(final Object key) {
+ return super.remove((String) key);
+ }
+
+ @Override
+ public void putAll(final Map<? extends String, ? extends T> m) {
+ if (isEmpty()) {
+ m.forEach(this::add);
+ } else {
+ m.forEach(this::put);
+ }
+ }
+
+ @Override
+ public Map<String, T> asShallowMap() {
Review Comment:
nitpick but place this up above implementing Map interfaces rather than
arbitrarily somewhere in the middle.
--
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: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
