dsmiley commented on code in PR #3235:
URL: https://github.com/apache/solr/pull/3235#discussion_r1980622308
##########
solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java:
##########
@@ -69,18 +69,16 @@ public String get(String param, String def) {
@Override
public void writeMap(EntryWriter ew) throws IOException {
- // TODO don't call toNamedList; more efficiently implement here
- // note: multiple values, if present, are a String[] under 1 key
- toNamedList()
- .forEach(
- (k, v) -> {
- if (v == null || "".equals(v)) return;
- try {
- ew.put(k, v);
- } catch (IOException e) {
- throw new RuntimeException("Error serializing", e);
- }
- });
+ for (Entry<String, String[]> entry : this) {
+ String[] value = entry.getValue();
+ // if only one value, don't wrap in an array
+ if (value.length == 1) {
+ assert value[0] != null;
Review Comment:
As an `assert`, it communicates that it shouldn't happen and it somewhat
enforces it (within tests, any way). I could further add a comment to say
"values should not be null" (above both conditions). On the single-value side,
this is just a cheap line. Perhaps 90% of parameters have a single value. I
don't want to bother with an assert of every value in the list; the brevity &
simplicity of that assert I kept is all I wanted.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
