cpoerschke commented on a change in pull request #169:
URL: https://github.com/apache/solr/pull/169#discussion_r651225518
##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/io/Tuple.java
##########
@@ -226,7 +214,7 @@ public Double getDouble(Object key) {
/**
* Return all tuple fields and their values.
*/
- public Map<Object, Object> getFields() {
+ public Map<String, Object> getFields() {
Review comment:
As a result of this change two casts in `UpdateStream` could be removed,
https://github.com/cpoerschke/solr/commit/d687763475aa43ee4e5fde652cdd05a581fe0fb9
illustrates.
##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/io/Tuple.java
##########
@@ -77,38 +77,26 @@ public Tuple() {
* A copy constructor.
* @param fields map containing keys and values to be copied to this tuple
*/
- public Tuple(Map<?, ?> fields) {
- for (Map.Entry<?, ?> entry : fields.entrySet()) {
- put(entry.getKey(), entry.getValue());
- }
+ public Tuple(Map<String, ?> fields) {
+ this.fields.putAll(fields);
+ EOF = this.fields.containsKey(StreamParams.EOF);
+ EXCEPTION = this.fields.containsKey(StreamParams.EXCEPTION);
}
- /**
- * Constructor that accepts an even number of arguments as key / value pairs.
- * @param fields a list of key / value pairs, with keys at odd and values at
- * even positions.
- */
- public Tuple(Object... fields) {
- if (fields == null) {
- return;
- }
- if ((fields.length % 2) != 0) {
- throw new RuntimeException("must have a matching number of key-value
pairs");
- }
- for (int i = 0; i < fields.length; i += 2) {
- // skip empty entries
- if (fields[i] == null) {
- continue;
- }
- put(fields[i], fields[i + 1]);
- }
+ public Tuple(String k1, Object v1) {
+ if (k1 != null) put(k1, v1);
+ }
+
+ public Tuple(String k1, Object v1, String k2, Object v2) {
+ if (k1 != null) put(k1, v1);
+ if (k2 != null) put(k2, v2);
}
public Object get(Object key) {
Review comment:
Thanks for explaining about the _"PECS (producer-extends,
consumer-super)"_ mnemonic!
So if I understood that right then that also explains why we don't change
`Object key` to `String key` here and in [other similar
methods](https://github.com/cpoerschke/solr/commit/220be7aa6ae36b8922f148ae72ab9b74f2478381)
right?
##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/io/Tuple.java
##########
@@ -77,38 +77,26 @@ public Tuple() {
* A copy constructor.
* @param fields map containing keys and values to be copied to this tuple
*/
- public Tuple(Map<?, ?> fields) {
- for (Map.Entry<?, ?> entry : fields.entrySet()) {
- put(entry.getKey(), entry.getValue());
- }
+ public Tuple(Map<String, ?> fields) {
+ this.fields.putAll(fields);
+ EOF = this.fields.containsKey(StreamParams.EOF);
+ EXCEPTION = this.fields.containsKey(StreamParams.EXCEPTION);
Review comment:
Am not 100% sure re: this refactor in the scenario where a deriving
class had overridden the `put` method, though haven't fully researched it and
might be confusing C++ and Java logic here too. Plus `put` method overriding
seems unlikely.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
