dsmiley commented on code in PR #3048:
URL: https://github.com/apache/solr/pull/3048#discussion_r1927992917
##########
solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java:
##########
@@ -68,24 +74,153 @@ 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;
}
/**
- * Returns a shared, empty, and immutable instance of SimpleOrderedMap.
+ * Returns {@code true} if this map maps one or more keys to the specified
value.
Review Comment:
"one or more" -- why more? Any way, don't feel obliged to re-specify
javadocs for interface implementations since it's automatically inherited.
##########
solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java:
##########
@@ -68,24 +74,153 @@ 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;
}
/**
- * Returns a shared, empty, and immutable instance of SimpleOrderedMap.
+ * Returns {@code true} if this map maps one or more keys to the specified
value.
*
- * @return Empty SimpleOrderedMap (immutable)
+ * @param value value whose presence in this map is to be tested
+ * @return {@code true} if this map maps one or more keys to the specified
value
*/
- public static SimpleOrderedMap<Object> of() {
- return EMPTY;
+ @Override
+ public boolean containsValue(final Object value) {
+ int sz = size();
+ for (int i = 0; i < sz; i++) {
+ T val = getVal(i);
+ if (Objects.equals(value, val)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * Has linear lookup time O(N)
+ *
+ * @see NamedList#get(String)
+ */
+ @Override
+ public T get(final Object key) {
+ return super.get((String) key);
}
/**
- * Returns an immutable instance of SimpleOrderedMap with a single key-value
pair.
+ * Associates the specified value with the specified key in this map If the
map previously
+ * contained a mapping for the key, the old value is replaced by the
specified value.
*
- * @return SimpleOrderedMap containing one key-value pair
+ * @param key key with which the specified value is to be associated value –
value to be
+ * associated with the specified key
+ * @param value to be associated with the specified key
+ * @return the previous value associated with key, or null if there was no
mapping for key. (A
+ * null return can also indicate that the map previously associated null
with 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;
+ }
+ }
+
+ /**
+ * @see NamedList#remove(String)
+ */
+ @Override
+ public T remove(final Object key) {
+ return super.remove((String) key);
+ }
+
+ /**
+ * Copies all of the mappings from the specified map to this map. These
mappings will replace any
+ * mappings that this map had for any of the keys currently in the specified
map.
+ *
+ * @param m mappings to be stored in this map
+ * @throws NullPointerException if the specified map is null
+ */
+ @Override
+ public void putAll(final Map<? extends String, ? extends T> m) {
+ m.forEach(this::put);
+ }
+
+ /**
+ * return a {@link Set} of all keys in the map.
+ *
+ * @return {@link Set} of all keys in the map
+ */
+ @Override
+ public Set<String> keySet() {
+ return new InnerMap().keySet();
Review Comment:
I think it'd be clearer to return a new AbstractList (and can declare the
return type to be List as well)
##########
solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java:
##########
@@ -68,24 +74,153 @@ 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;
}
/**
- * Returns a shared, empty, and immutable instance of SimpleOrderedMap.
+ * Returns {@code true} if this map maps one or more keys to the specified
value.
*
- * @return Empty SimpleOrderedMap (immutable)
+ * @param value value whose presence in this map is to be tested
+ * @return {@code true} if this map maps one or more keys to the specified
value
*/
- public static SimpleOrderedMap<Object> of() {
- return EMPTY;
+ @Override
+ public boolean containsValue(final Object value) {
+ int sz = size();
+ for (int i = 0; i < sz; i++) {
+ T val = getVal(i);
+ if (Objects.equals(value, val)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * Has linear lookup time O(N)
+ *
+ * @see NamedList#get(String)
+ */
+ @Override
+ public T get(final Object key) {
+ return super.get((String) key);
}
/**
- * Returns an immutable instance of SimpleOrderedMap with a single key-value
pair.
+ * Associates the specified value with the specified key in this map If the
map previously
+ * contained a mapping for the key, the old value is replaced by the
specified value.
*
- * @return SimpleOrderedMap containing one key-value pair
+ * @param key key with which the specified value is to be associated value –
value to be
+ * associated with the specified key
+ * @param value to be associated with the specified key
+ * @return the previous value associated with key, or null if there was no
mapping for key. (A
+ * null return can also indicate that the map previously associated null
with 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;
+ }
+ }
+
+ /**
+ * @see NamedList#remove(String)
+ */
+ @Override
+ public T remove(final Object key) {
+ return super.remove((String) key);
+ }
+
+ /**
+ * Copies all of the mappings from the specified map to this map. These
mappings will replace any
+ * mappings that this map had for any of the keys currently in the specified
map.
+ *
+ * @param m mappings to be stored in this map
+ * @throws NullPointerException if the specified map is null
+ */
+ @Override
+ public void putAll(final Map<? extends String, ? extends T> m) {
+ m.forEach(this::put);
+ }
+
+ /**
+ * return a {@link Set} of all keys in the map.
+ *
+ * @return {@link Set} of all keys in the map
+ */
+ @Override
+ public Set<String> keySet() {
+ return new InnerMap().keySet();
+ }
+
+ /**
+ * return a {@link Collection} of all values in the map.
+ *
+ * @return {@link Collection} of all values in the map
+ */
+ @Override
+ public Collection<T> values() {
+ return new InnerMap().values();
Review Comment:
I think it'd be clearer to return a new AbstractList (and can declare the
return type to be List as well)
##########
solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java:
##########
@@ -68,24 +74,153 @@ 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;
}
/**
- * Returns a shared, empty, and immutable instance of SimpleOrderedMap.
+ * Returns {@code true} if this map maps one or more keys to the specified
value.
*
- * @return Empty SimpleOrderedMap (immutable)
+ * @param value value whose presence in this map is to be tested
+ * @return {@code true} if this map maps one or more keys to the specified
value
*/
- public static SimpleOrderedMap<Object> of() {
- return EMPTY;
+ @Override
+ public boolean containsValue(final Object value) {
+ int sz = size();
+ for (int i = 0; i < sz; i++) {
+ T val = getVal(i);
+ if (Objects.equals(value, val)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * Has linear lookup time O(N)
+ *
+ * @see NamedList#get(String)
+ */
+ @Override
+ public T get(final Object key) {
+ return super.get((String) key);
}
/**
- * Returns an immutable instance of SimpleOrderedMap with a single key-value
pair.
+ * Associates the specified value with the specified key in this map If the
map previously
+ * contained a mapping for the key, the old value is replaced by the
specified value.
*
- * @return SimpleOrderedMap containing one key-value pair
+ * @param key key with which the specified value is to be associated value –
value to be
+ * associated with the specified key
+ * @param value to be associated with the specified key
+ * @return the previous value associated with key, or null if there was no
mapping for key. (A
+ * null return can also indicate that the map previously associated null
with 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;
+ }
+ }
+
+ /**
+ * @see NamedList#remove(String)
+ */
+ @Override
+ public T remove(final Object key) {
+ return super.remove((String) key);
+ }
+
+ /**
+ * Copies all of the mappings from the specified map to this map. These
mappings will replace any
+ * mappings that this map had for any of the keys currently in the specified
map.
+ *
+ * @param m mappings to be stored in this map
+ * @throws NullPointerException if the specified map is null
+ */
+ @Override
+ public void putAll(final Map<? extends String, ? extends T> m) {
+ m.forEach(this::put);
+ }
+
+ /**
+ * return a {@link Set} of all keys in the map.
+ *
+ * @return {@link Set} of all keys in the map
+ */
+ @Override
+ public Set<String> keySet() {
+ return new InnerMap().keySet();
+ }
+
+ /**
+ * return a {@link Collection} of all values in the map.
+ *
+ * @return {@link Collection} of all values in the map
+ */
+ @Override
+ public Collection<T> values() {
+ return new InnerMap().values();
+ }
+
+ /**
+ * Returns a {@link Set} view of the mappings contained in this map.
+ *
+ * @return {@link Set} view of mappings
+ */
+ @Override
+ public Set<Entry<String, T>> entrySet() {
+
+ return new AbstractSet<>() {
+ @Override
+ public Iterator<Entry<String, T>> iterator() {
+ return SimpleOrderedMap.this.iterator();
+ }
+
+ @Override
+ public int size() {
+ return SimpleOrderedMap.this.size();
+ }
+ };
+ }
+
+ /**
+ * Returns an immutable instance of {@link SimpleOrderedMap} with a single
key-value pair.
+ *
+ * @return {@link SimpleOrderedMap} containing one key-value pair
*/
public static <T> SimpleOrderedMap<T> of(String name, T val) {
- return new SimpleOrderedMap<>(List.of(name, val));
+ return new SimpleOrderedMap<T>(List.of(name, val));
+ }
+
+ /**
+ * Returns a shared, empty, and immutable instance of {@link
SimpleOrderedMap}.
+ *
+ * @return Empty {@link SimpleOrderedMap} (immutable)
+ */
+ public static SimpleOrderedMap<Object> of() {
+ return EMPTY;
+ }
+
+ private class InnerMap extends AbstractMap<String, T> {
Review Comment:
If you follow my feedback above, we won't need this.
--
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
